-
Notifications
You must be signed in to change notification settings - Fork 24
feat: add ts types #72
Conversation
d8266ec to
d8eaa89
Compare
Gozala
left a comment
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.
Mostly nits & suggestions. Only thing I do feel strongly about is type names that I think should be singular instead of plural.
I also flagged out few things with
With type names changed 👍from me and no need for another review cycle.
| ``` | ||
|
|
||
| ## API | ||
| https://multiformats.github.io/js-multibase/ |
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.
💭 Maybe we do not need the whole section just for the link ?
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.
Suggestions ?
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.
I don’t know maybe API Docs badge at the top ?
achingbrain
left a comment
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.
LGTM, just needs the gh url for aegir removing.
Co-authored-by: Irakli Gozalishvili <[email protected]>
c748d31 to
280b133
Compare
needs ipfs/aegir#671
closes #54
closes #29
check published docs https://multiformats.github.io/js-multibase