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

Consolidate media types to application/did. #868

Merged
merged 3 commits into from
Nov 15, 2024
Merged

Conversation

msporny
Copy link
Member

@msporny msporny commented Oct 30, 2024

This PR is an attempt to address issue #863 by registering a single media type for both the JSON and JSON-LD serializations of the core data model.


Preview | Diff

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.

lgtm

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.

In the section

Person & email address to contact for further information:

I think my name should be changed to the Working Group references.

(Github does not give me access to that section, so I cannot put in a change request)

@msporny
Copy link
Member Author

msporny commented Nov 2, 2024

I think my name should be changed to the Working Group references.

Fixed in 653364c

Copy link
Contributor

@pchampin pchampin left a comment

Choose a reason for hiding this comment

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

In addition to my comment on the self-referential normative dependency,

line 910 has a Respec-managed internal reference <a href="#application-did-ld-json"></a> which is now broken and should be adapted

@@ -5597,7 +5598,8 @@ <h2>application/did+json</h2>
</dd>
<dt>Security considerations:</dt>
<dd>
See <a data-cite="RFC8259#section-12">RFC&nbsp;8259, section 12</a> [[RFC8259]].
See <a data-cite="DID-CORE#security-considerations">DID Core v1.1, Security Considerations</a>
[[DID-CORE]].
Copy link
Contributor

@pchampin pchampin Nov 6, 2024

Choose a reason for hiding this comment

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

Suggested change
[[DID-CORE]].
.

The [[ ... ]] reference creates an entry in the Normative References, which makes this specification normatively depend on itself. This is strange and should probably be avoided.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't say "this specification" because these are instructions to IANA (they're supposed to stand on their own). I just removed the reference to address the concern you mentioned since we have a link on the previous line..

Copy link
Member Author

Choose a reason for hiding this comment

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

line 910 has a Respec-managed internal reference which is now broken and should be adapted

Fixed in d6a7bf6.

@pchampin
Copy link
Contributor

This was discussed during the did meeting on 14 November 2024.

View the transcript

w3c/did-core#868

manu: Discussion about Issue 863 -- can we use a single media type. Lots of IETF / IANA discussions, but agreed on one media type "DID", for JSON, and then if CBOR or other is needed, add that DID+CBOR.
… any objections?
… none - yay!


@msporny
Copy link
Member Author

msporny commented Nov 15, 2024

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

@msporny msporny merged commit 308b4c1 into main Nov 15, 2024
1 check passed
@msporny msporny deleted the msporny-media-type branch November 15, 2024 15:42
@msporny msporny restored the msporny-media-type branch November 15, 2024 15:48
@msporny msporny deleted the msporny-media-type branch November 15, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
class 3 Other changes that do not add new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants