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

Fix editorial and substantive issues in JSON Production and Consumption section. #653

Merged
merged 5 commits into from
Feb 17, 2021

Conversation

msporny
Copy link
Member

@msporny msporny commented Feb 15, 2021

This PR attempts to clean up the JSON Production and Consumption section in a way that enables the data model to be serialized for data structures other than a DID Document (to align it with the Resolution serialization requirements), while keeping the DID Document serialization rules more or less the same. There was wierdness related to tying the resolution section in normatively to the production/consumption rules, which really should be done higher up in the stack. I'm marking it as substantive because, while I'm fairly certain implementations wouldn't be affected by the changes, normative language was changed/merged/removed.


Preview | Diff

@msporny msporny changed the title Fix editorial and substantive issues in JSON Production and Consumptio section. Fix editorial and substantive issues in JSON Production and Consumption section. Feb 15, 2021
@msporny msporny added the substantive This is a substantive change to the specification. label Feb 15, 2021
index.html Outdated
additional processing applied to its value, which will be added verbatim to the
<a href="#data-model">data model</a>.
If media type information is available to a <a>conforming consumer</a> and the
media type value is <code>application/did+json</code>, the data structure being
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
media type value is <code>application/did+json</code>, the data structure being
media type value is <code>application/did+json</code>: the data structure being

Copy link
Member

Choose a reason for hiding this comment

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

I think this change would be an error. See mine below. https://github.com/w3c/did-core/pull/653/files?diff=unified&w=1#r577047614

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied change from @TallTed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Woops, @TallTed'd changes were wrong... @rhiaro's were correct... changed back via another change request.

index.html Outdated
member of the JSON object. The values of entries,
including all extensions, are encoded in JSON [[RFC8259]] by mapping entry
values to JSON types as follows:
All data structures expressed by the <a href="#data-model">data model</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

This change https://github.com/w3c/did-core/pull/653/files#r577646064 implements https://www.w3.org/2021/02/16-did-topic-minutes.html#r01. @peacekeeper, I'm making the same changes to the other PRs... I assume this addresses your concern to the point where you'll approve merging the section?

@msporny msporny requested a review from peacekeeper February 17, 2021 14:19
Copy link
Contributor

@peacekeeper peacekeeper left a comment

Choose a reason for hiding this comment

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

@msporny
Copy link
Member Author

msporny commented Feb 17, 2021

Substantive, multiple reviews, changes requested and made, objection raised by @peacekeeper and addressed via specification text change, no sustained objections, merging.

@msporny msporny merged commit 7a2b2dc into main Feb 17, 2021
@msporny msporny deleted the msporny-cr-json-representation branch February 21, 2021 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
substantive This is a substantive change to the specification.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants