-
Notifications
You must be signed in to change notification settings - Fork 22
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
fix!: use MultihashDigest type in stores #1474
Conversation
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 mark this as breaking change :)
@@ -126,21 +124,17 @@ export class BlobsStorage { | |||
this.content = content | |||
} | |||
|
|||
/** @param {import('multiformats').MultihashDigest} digest */ | |||
/** @param {Types.MultihashDigest} digest */ | |||
#bucketPath(digest) { |
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.
nit: could we call this param multihashDigest everywhere? it feels weird now, and we have some digest.digest
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.
So verbose though, and we very rarely use digest.digest
... 😄
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.
mhDigest
instead? 😅
anyway, I don't strongly oppose if you want to keep as this
739e02c
to
d9bd509
Compare
🤖 I have created a release *beep* *boop* --- ## [18.0.0](upload-api-v17.1.0...upload-api-v18.0.0) (2024-06-07) ### ⚠ BREAKING CHANGES * `AllocationsStorage` and `BlobsStorage` methods not take `MultihashDigest` types instead of `Uint8Array`s. ### Features * publish index claim ([#1487](#1487)) ([237b0c6](237b0c6)) ### Fixes * stop writing to DUDEWHERE ([#1500](#1500)) ([cf0a1d6](cf0a1d6)) * use MultihashDigest type in stores ([#1474](#1474)) ([6c6a3bd](6c6a3bd)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [15.0.0](w3up-client-v14.1.1...w3up-client-v15.0.0) (2024-06-13) ### ⚠ BREAKING CHANGES * `AllocationsStorage` and `BlobsStorage` methods not take `MultihashDigest` types instead of `Uint8Array`s. ### Features * utility exports for better UX ([#1505](#1505)) ([54b0d93](54b0d93)) ### Fixes * use MultihashDigest type in stores ([#1474](#1474)) ([6c6a3bd](6c6a3bd)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This forces callers to decode
Uint8Arrays
that may come from user input before the storage method is called and that means we don't just use them without validation that they are actually a multihash.BREAKING CHANGE:
AllocationsStorage
andBlobsStorage
methods not takeMultihashDigest
types instead ofUint8Array
s.