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

Organize v2 context. #1511

Merged
merged 1 commit into from
Jul 6, 2024
Merged

Organize v2 context. #1511

merged 1 commit into from
Jul 6, 2024

Conversation

davidlehn
Copy link
Contributor

@davidlehn davidlehn commented Jun 25, 2024

The v2 context has been written and updated by hand over time and some of the ordering and formatting was not following a strict pattern. This attempts to clean it up to be easier to read. The format here is somewhat arbitrary, and other formats are possible, but this seems easy to work with. This organization was done by hand.

  • Opinionated but consistent organization and sorting.
  • Semantically equivalent to data in previous style.
  • Order is recursive and roughly follows a pattern:
    • JSON-LD keywords
    • blank line
    • id/type terms
    • blank line
    • jose/jwt property terms
      • sorted
    • blank line
    • other property terms
      • sorted
    • blank line
    • type terms
      • sorted
      • for TypeCredential/Type pairs, put TypeCredential first
      • blank line between each
  • Drop down and indent values to try and be under 80 chars per line.

(Note this includes commits from #1510 due to github limitations and will be rebased if/when that is merged.) (Branch was rebased.)

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 for taking the time to do this

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.

Can we move the JWT properties to the bottom, it's confusing to hit a bunch of JOSE properties for a page and a half before you get to any of the VC v2 properties.

We will probably want to put imported securing mechanism properties, including proof and DataIntegrityProof, towards the bottom of the document.

@msporny msporny added editorial Purely editorial changes to the specification. CR1 This item was processed during CR1 labels Jun 27, 2024
@davidlehn
Copy link
Contributor Author

Can we move the JWT properties to the bottom, it's confusing to hit a bunch of JOSE properties for a page and a half before you get to any of the VC v2 properties.

We will probably want to put imported securing mechanism properties, including proof and DataIntegrityProof, towards the bottom of the document.

Other orderings are certainly possible. This change is for humans since the machines will read it the same way. The aim of this first pass was primarily a few major groupings then alphabetical ordering. That matches how I've been trying to read the file in the past and the motivation for this change. While I understand the idea of further grouping, it starts to again make it difficult to find properties and types. I had thought about putting core properties/types first then other specialized ones. But at first glance it didn't seem that great. Open to other thoughts on this. I can adapt to whatever order is used. At least some ordering is an improvement.

And I'll note that "..." is confusing no matter where it is.

- Opinionated but consistent organization and sorting.
- Semantically equivalent to previous style.
- Order is recursive and roughly follows a pattern:
  - JSON-LD keywords
  - blank line
  - id/type terms
  - blank line
  - jose/jwt property terms
    - sorted
  - blank line
  - other property terms
    - sorted
  - blank line
  - type terms
    - sorted
    - for TypeCredential/Type pairs, put TypeCredential first
    - blank line between each
- Drop down and indent values to try and be under 80 chars per line.
@msporny
Copy link
Member

msporny commented Jul 6, 2024

Editorial, multiple reviews, changes requested (and no made, but @msporny isn't going to object over them), no objections, merging.

@msporny msporny merged commit b193f3f into w3c:main Jul 6, 2024
1 check passed
@msporny
Copy link
Member

msporny commented Jul 6, 2024

@davidlehn wrote:

I had thought about putting core properties/types first then other specialized ones. But at first glance it didn't seem that great. Open to other thoughts on this.

I took what you did and regrouped with the following layout:

  1. All properties are in alphabetical order.
  2. Classes are next, which are grouped by "most likely to be used to less likely to be used" order -- e.g., among all of the classes, "VerifiableCredential" is most likely to be used, then "VerifiablePresentation", then "JsonSchemaCredential", and so on. This optimizes for developers trying to find stuff based on a visual scan from the top of the file... if they don't find what they're looking for, they can do a text search for their term (and frankly, the file is big enough that you're probably going to do a search if you don't just scan from the top to find what you want).
  3. External classes follow that (e.g., BitstringStatusList)
  4. Securing mechanisms are listed at the bottom.

@davidlehn davidlehn deleted the organize-context branch July 8, 2024 19:28
@davidlehn
Copy link
Contributor Author

@msporny Minor nit with your reordering commit (which wasn't in a PR I think?). Is dropping the trailing newline intentional? This is one of those things that tends to cause some churn due to editor settings and it's inconsistent between projects. In this case it matters since the file will be hashed. I'm on team trailing newline. A .editorconfig might help for this, for those that use that tooling.

@msporny
Copy link
Member

msporny commented Jul 8, 2024

@msporny Minor nit with your reordering commit (which wasn't in a PR I think?). Is dropping the trailing newline intentional? This is one of those things that tends to cause some churn due to editor settings and it's inconsistent between projects. In this case it matters since the file will be hashed. I'm on team trailing newline. A .editorconfig might help for this, for those that use that tooling.

Nope, the deletion of the trailing new line was inadvertent. We can add it back in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR1 This item was processed during CR1 editorial Purely editorial changes to the specification.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants