-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: move args for createEncoder
into non-repeating options
& fix typedoc
#1146
Conversation
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to update changelog to document breaking api change
contentTopic: string, | ||
ephemeral = false | ||
): Encoder { | ||
export function createEncoder({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we think we want to accept string | EncoderOptions
so most of the time, the developer just pass the content topic string
and only pass an object
in advance usage?
WDYT @hackyguru ?
@@ -63,6 +64,13 @@ export class Encoder implements IEncoder { | |||
} | |||
} | |||
|
|||
export interface EciesEncoderOptions extends EncoderOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we are in the ecies
package so the Ecies
prefix is not needed.
@@ -61,6 +62,13 @@ export class Encoder implements IEncoder { | |||
} | |||
} | |||
|
|||
export interface SymmetricEncoderOptions extends EncoderOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we are in the symmetric
package so the Symmetric
prefix is not needed.
- update changelog - change naming for `EciesEncoderOptions` and `SymmetricEncoderOptions`
@fryorcraken addressed the above ^ |
* address comments from #1145 * fix: typedoc * address comments in #1146 (review) - update changelog - change naming for `EciesEncoderOptions` and `SymmetricEncoderOptions`
ref: #1010 (comment)