Conversation
packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/types.ts
Show resolved
Hide resolved
| return await this.#sdk.signMessage(message); | ||
| } | ||
|
|
||
| async pairIdentifiers(pairing: Pair[]): Promise<void> { |
There was a problem hiding this comment.
Hmmm...
I'm hesitant if implementations should be done in this combined class vs in the individual implementations.
Context:
- Combined Class: 1 class; but poor type interface
- Individual Class: the developer can choose a specific implementation & has great type interface.
There was a problem hiding this comment.
Yes lets move the implementation to the individual classes, that way the better typed implementations can also utilise this method.
Either do this now, or in a separate implementation.
There was a problem hiding this comment.
Only annoying thing is that the implementation is identical on both implementations, so a bit redundant. (Sadly JS does not support the trait system lol).
Even though there would be duplication, I think it is nicer to have that abstraction.
Prithpal-Sooriya
left a comment
There was a problem hiding this comment.
LGTM.
1 comment on adding the implementation to the grouped class.
Not against it, but originally that grouped class was supposed to be a thin wrapper around the individual implementations.
We can refactor this in another PR.
Explanation
Adding pairing profile functionality to the profile sync SDK.
@metamask/profile-sync-controller/sdkChecklist