-
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: upload/add capability root validation #136
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.
LGTM
We should also look into extending the array (and maybe some other validators) to have minimum length, etc?
Something like, in the todo app example, could be like "item name" as an nb, and needs to be
string.minimumLength(4);
Somewhat unrelated, but just should maybe think about it
Correct! Thanks for catching and fixing |
🤖 I have created a release *beep* *boop* --- ## [4.0.2](access-v4.0.1...access-v4.0.2) (2022-11-04) ### Bug Fixes * upload/add capability root validation ([#136](#136)) ([aae5b66](aae5b66)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
making root required sounds good! It was only optional because of some bug in ucanto which has been fixed. As of As a side it may be a good idea to have separate field for the root shard so we won’t have to check every single one |
🤖 I have created a release *beep* *boop* --- ## [4.0.2](access-v4.0.1...access-v4.0.2) (2022-11-04) ### Bug Fixes * upload/add capability root validation ([#136](#136)) ([9267885](9267885)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@Gozala should
root
be the root CID of the data, not a CAR CID? This is how it's implemented AFAIK. Or were you thinking it was the CID of the CAR that has the root data CID?This PR switches
root
to allow any CID, not just a CAR CID.Also, for a separate PR, maybe neither of these fields should be optional. Like, an "upload" should always have a data root CID and be present in at least 1 shard?