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

Added section about W3C Verifiable Credential/Presentation representation #185

Merged
merged 11 commits into from
Dec 7, 2023

Conversation

Artemkaaas
Copy link
Contributor

@Artemkaaas Artemkaaas commented Nov 15, 2023

Add section describing how AnonCreds credentials and presentations can be represented in the W3C Data Model standard.

spec/w3c_representation.md Outdated Show resolved Hide resolved
spec/w3c_representation.md Outdated Show resolved Hide resolved
spec/w3c_representation.md Show resolved Hide resolved
spec/w3c_representation.md Outdated Show resolved Hide resolved
spec/w3c_representation.md Outdated Show resolved Hide resolved
@swcurran
Copy link
Member

@Artemkaaas -- please address the conflicts. Should be easy :-)

Signed-off-by: artem.ivanov <[email protected]>
Signed-off-by: artem.ivanov <[email protected]>
@swcurran
Copy link
Member

swcurran commented Dec 4, 2023

Looks good!

I think there are some style adjustments to be made -- e.g. when/where to use [[ref:]]-- to be consistent with the rest of the spec. However, I suggest that since I'm about to do a pass to align that across the spec, I can do that in a follow up PR. I also saw a couple of cleanups, but I think they can be adjusted during that edit cleanup pass that is planned.

The only thing that I think is under explained is the use of MessagePack in the spec. I think there should be a sub-section and overview of how MessagePack is used, and at least a link to the MessagePack spec.

@@ -130,13 +130,6 @@ schema in order to include the information about AnonCreds related definitions t
* `type` - `AnonCredsDefinition`
* `schema` - id of [[ref: Schema]]
* `definition` - id of [[ref: Credential Definition]]
* `revocation_registry` - (Optional) id of [[ref: Revocation Registry Definition]]
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to remove revocation_registry as well here? Or was that not supposed to be there, since it is not in the example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems you look outdated version. revocation_registry is not there anymore.

Copy link
Member

Choose a reason for hiding this comment

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

That’s why I commented. This commit removed the encoding everywhere else, but in this one place it also removed the “revocation_registry” line — different from the intent of the commit. I’m sure it is fine, but just wanted to make sure you intended to do that. Sounds like you did.

@swcurran swcurran merged commit d23d94b into hyperledger:main Dec 7, 2023
1 check passed
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.

4 participants