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.

Sign up for GitHub

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

Jump to bottom

Editorial update index.html: use JsonSchema and not JsonSchema2023 #1225

Closed
wants to merge 8 commits into from
Closed

Editorial update index.html: use JsonSchema and not JsonSchema2023 #1225

wants to merge 8 commits into from

Conversation

iherman
Copy link
Member

@iherman iherman commented Aug 3, 2023 •
edited by pr-preview bot
Loading

This is just a leftover of the naming/versioning change around JSON Schemas.

(Editorial change, only in the examples).

Preview | Diff

msporny and others added 7 commits August 2, 2023 10:21
@msporny @TallTed
@dlongley @jandrieu @Sakurann
Add section on Ecosystem Compatibility. (#1203)
22e3730 
17 approvals, open for more than a week, no objections during the Special topic call on 08-01-2023

* Add section on Ecosystem Compatibility.

* Fix grammar in Ecosystem Compatibility section.

Co-authored-by: Ted Thibodeau Jr <[email protected]>

* Add references to "digital credentials" specifications.

* Update Ecosystem Compatibility based on VCWG Special Topic call.

* Add clarification around when a transformation becomes a VC.

* Add note about what constitutes a verifiable credential.

* Clarify that a conforming document has two possible media types.

* Clarify that JOSE, COSE, wrapped conforming documents are VCs/VPs.

* Clarify which specs need to follow the ecosystem guidelines.

Co-authored-by: Dave Longley <[email protected]>

* Remove vague statement about "any securing mechanism" per @jandrieu.

Co-authored-by: Joe Andrieu <[email protected]>

* Apply editorial suggestions from @Sakurann to Ecosystem Compatibility.

Co-authored-by: Kristina <[email protected]>

---------

Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Dave Longley <[email protected]>
Co-authored-by: Joe Andrieu <[email protected]>
Co-authored-by: Kristina <[email protected]>
@OR13 @brentzundel
Update reservation table
6592f3b
@OR13 @brentzundel
Update contexts/credentials/v2
0ca0f70
@msporny @brentzundel
Update URL to add confidenceMethod specification.
839c3b4
@OR13 @TallTed @brentzundel
Update index.html
f3e2b30 
Co-authored-by: Ted Thibodeau Jr <[email protected]>
@brentzundel
Update index.html
cf429b1 
minor typo fix.
@iherman
Editorial update index.html: use JsonSchema and not JsonSchema2023
af67521 
This is just a leftover of the naming/versioning change around JSON Schemas.

(Editorial change, only in the examples).
@iherman iherman added editorial Purely editorial changes to the specification. before CR labels Aug 3, 2023
decentralgabe
decentralgabe reviewed Aug 3, 2023
View reviewed changes
index.html Outdated Show resolved Hide resolved
decentralgabe
decentralgabe reviewed Aug 3, 2023
View reviewed changes
index.html Outdated Show resolved Hide resolved
decentralgabe
decentralgabe reviewed Aug 3, 2023
View reviewed changes
index.html Outdated Show resolved Hide resolved
decentralgabe
decentralgabe approved these changes Aug 3, 2023
View reviewed changes
Copy link
Contributor

@decentralgabe decentralgabe left a comment

Choose a reason for hiding this comment

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

thanks!

OR13
OR13 approved these changes Aug 3, 2023
View reviewed changes
Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

Assuming suggestions are applied

dmitrizagidulin
dmitrizagidulin approved these changes Aug 4, 2023
View reviewed changes
Copy link
Contributor

@dmitrizagidulin dmitrizagidulin left a comment

Choose a reason for hiding this comment

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

(The Ecosystem Compatibility section seems to be an artifact of stray commit history)

msporny
msporny requested changes Aug 4, 2023
View reviewed changes
Copy link
Member

@msporny msporny left a comment

Choose a reason for hiding this comment

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

PR includes commits on main, needs a rebase.

@iherman
Copy link
Member Author

iherman commented Aug 4, 2023

PR includes commits on main, needs a rebase.

@msporny I do not understand this remark. Rebase on what?

@msporny
Copy link
Member

msporny commented Aug 4, 2023

@msporny I do not understand this remark. Rebase on what?

Look at the file diff... it contains your changes and a bunch of changes on an invalid branch on main. There was a PR that was pulled in a few days ago (the Ecosystem Compatability PR) that destroyed history, so I had to re-create history on main and force push to main, which then caused this PR (and many others) to have issues. To rebase:

git checkout main
git pull
git checkout iherman-patch-1
git rebase main

^^ That might do the trick, or you might have conflicts you need to resolve.

I can also squash-merge, but the current diff is probably going to confuse people that are reviewing, and they'll start suggesting changes to things that were not in this PR.

This was referenced Aug 4, 2023
Closed
Merged
@iherman
Copy link
Member Author

iherman commented Aug 4, 2023 •
edited
Loading

Moving over to #1229; this PR should be closed without merge.

@TallTed
Copy link
Member

TallTed commented Aug 4, 2023

Moving over to #1229; this PR should be closed without merge.

As with #1226, maybe change title so people don't get drawn down the glitch rabbit hole?

@decentralgabe decentralgabe mentioned this pull request Aug 7, 2023
Closed
@brentzundel
Copy link
Member

When #1229 is merged, this PR should be closed.

@brentzundel brentzundel added pending close Close if no objection within 7 days and removed editorial Purely editorial changes to the specification. before CR labels Aug 7, 2023
awoie
awoie suggested changes Aug 9, 2023
View reviewed changes
Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

contains ecosystem compatibility which imo is not desired

@brentzundel
Copy link
Member

@awoie this PR has been superseded by #1229 and will be closed when that PR is merged.

@msporny
Copy link
Member

msporny commented Aug 9, 2023 •
edited
Loading

Merged via #1229 (which implements the text in this PR but with a more recent branch from main), closing.

@msporny msporny closed this Aug 9, 2023
@msporny msporny deleted the iherman-patch-1 branch November 11, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msporny msporny msporny requested changes

@awoie awoie awoie requested changes

@dmitrizagidulin dmitrizagidulin dmitrizagidulin approved these changes

@andresuribe87 andresuribe87 andresuribe87 approved these changes

@decentralgabe decentralgabe decentralgabe approved these changes

@OR13 OR13 OR13 approved these changes

@selfissued selfissued Awaiting requested review from selfissued selfissued is a code owner

Assignees
No one assigned
Labels
pending close Close if no objection within 7 days
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@iherman @msporny @TallTed @brentzundel @dmitrizagidulin @andresuribe87 @decentralgabe @OR13 @awoie