-
Notifications
You must be signed in to change notification settings - Fork 39
refactor: base32 encode CIDs by default #73
Conversation
BREAKING CHANGE: `toString` and `toBaseEncodedString` will now use base32 encoding to encode version 1 CIDs by default. License: MIT Signed-off-by: Alan Shaw <[email protected]>
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'd like to get #72 merged first, which isn't a breaking change (as this change might need a lot of code changes).
@@ -110,7 +110,7 @@ describe('CID', () => { | |||
expect(cid).to.have.property('version', 1) | |||
expect(cid).to.have.property('multihash') | |||
|
|||
expect(cid.toBaseEncodedString()).to.be.eql(cidStr) | |||
expect(cid.toBaseEncodedString('base58btc')).to.be.eql(cidStr) |
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.
Please change the expect
to use no base parameter, else the test doesn't match its description and is the same test as the one above.
Of course - there's absolutely no rush right now on getting this merged and released - I'll ping here when ready. |
Good to know. Without any context I was under the impression it's one of those "last missing pieces" :) |
No longer required - v1 CIDs already switched to using base32 encoding by default. |
BREAKING CHANGE:
toString
andtoBaseEncodedString
will now use base32 encoding to encode version 1 CIDs by default.