Skip to content
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

Make all Multicodec / Multibase references non-normative. #42

Merged
merged 3 commits into from
Oct 12, 2023

Conversation

msporny
Copy link
Member

@msporny msporny commented Sep 27, 2023

This PR attempts to address issue #39 by making all Multicodec / Multibase references non-normative and cites the normative sections of PR w3c/vc-data-integrity#196 as requested by the issue submitter.


Preview | Diff

Copy link

@selfissued selfissued left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR doesn't do the job. The point of the issue was to remove the references - not to make them non-normative. Making them non-normative means that required fields are undefined, which is unacceptable for a standard.

Instead, remove the references to multibase and add normative text fully defining the contents of the fields.

Copy link
Member

@iherman iherman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment in w3c/vc-di-eddsa#63; it is verbatim relevant for this PR, too

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small stuff

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@iherman
Copy link
Member

iherman commented Sep 28, 2023

My original review comment is moot, in view of w3c/vc-data-integrity#196 (comment). Changing the review.

@iherman iherman self-requested a review September 28, 2023 01:36
@msporny
Copy link
Member Author

msporny commented Sep 30, 2023

@selfissued there are now no references to the IETF I-Ds on Multiformats, we instead either normatively define the values needed in the spec, or refer back to VC Data Integrity, which defines the values needed. Requesting re-review.

Copy link

@selfissued selfissued left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This spec is still referencing ?MULTIBASE rather than normative definitions of the features used.

Many of the comments in w3c/vc-di-eddsa#63 (review) and w3c/vc-data-integrity#196 (review) also apply to this specification.

In particular, the referenced terms need be normatively defined in sufficient detail to enable interoperable implementations.

Copy link
Contributor

@seabass-labrax seabass-labrax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this resolves all the concerns that were brought up in this week's VCWG meeting.

@msporny
Copy link
Member Author

msporny commented Oct 6, 2023

This spec is still referencing ?MULTIBASE rather than normative definitions of the features used.

Sorry, there was code stuck on my local copy that removed all those references, requesting another re-review from you.

@msporny
Copy link
Member Author

msporny commented Oct 11, 2023

@selfissued re-ping to re-review... ideally, you approve before we merge this. It's all green on reviews so far.

Copy link

@selfissued selfissued left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this.

@iherman
Copy link
Member

iherman commented Oct 12, 2023

The issue was discussed in a meeting on 2023-10-11

  • no resolutions were taken
View the transcript

1.3. Make all Multicodec / Multibase references non-normative. (pr vc-di-ecdsa#42)

See github pull request vc-di-ecdsa#42.

Manu Sporny: I see he's approved the last remaining well as well.
… That is great, that unblocks us across the board for all DI and cryptosuite specs.
… That's that item, we can shift back to VCDM.

Brent Zundel: There are a lot of VCDM PRs.

@msporny
Copy link
Member Author

msporny commented Oct 12, 2023

Normative, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit 4727989 into main Oct 12, 2023
2 checks passed
@msporny msporny deleted the msporny-mf-non-norm branch October 12, 2023 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet