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

Standardize resolution for "empty". #77

Merged
merged 4 commits into from
Jan 9, 2024
Merged

Standardize resolution for "empty". #77

merged 4 commits into from
Jan 9, 2024

Conversation

andresuribe87
Copy link
Contributor

@andresuribe87 andresuribe87 commented Dec 13, 2023 •
edited
Loading

Fixes #76 by making sure all fields are present.

  • When empty, didDocument is set to null.
  • When empty, other fields (like didDocumentMetadata and didResolutionMetadata) are set to {}

@frankhinek
Copy link
Contributor

@andresuribe87 This should be errors: true?

https://github.com/TBD54566975/sdk-development/pull/77/files#diff-318c4b5bc4436fbd65a1c336bb5667ebaa012c6e805093b51b18889cc925d6edR104

@andresuribe87
Copy link
Contributor Author

@andresuribe87 This should be errors: true?

https://github.com/TBD54566975/sdk-development/pull/77/files#diff-318c4b5bc4436fbd65a1c336bb5667ebaa012c6e805093b51b18889cc925d6edR104

Done in c72bfbb

@mistermoe
Copy link
Member

interpretation of didDocument empty is null not an empty object. Reference

@andresuribe87 andresuribe87 mentioned this pull request Jan 8, 2024
Closed
@andresuribe87 andresuribe87 changed the title Standardize resolution to use {} for empty. Standardize resolution for "empty". Jan 8, 2024
@andresuribe87
Copy link
Contributor Author

andresuribe87 commented Jan 8, 2024 •
edited
Loading

@mistermoe please take another look. What I think you were requesting was done in 98c8dc2

Let me know if that wasn't the case.

mistermoe
mistermoe approved these changes Jan 9, 2024
View reviewed changes
Copy link
Member

@mistermoe mistermoe left a comment

Choose a reason for hiding this comment

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

💯

@andresuribe87 andresuribe87 merged commit 6b3ef98 into main Jan 9, 2024
6 checks passed
@andresuribe87 andresuribe87 deleted the issue_76 branch January 9, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mistermoe mistermoe mistermoe approved these changes

@nitro-neal nitro-neal nitro-neal approved these changes

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

@phoebe-lew phoebe-lew Awaiting requested review from phoebe-lew

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

@amika-sq amika-sq Awaiting requested review from amika-sq

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Question about DID Resolution Test Vectors
4 participants
@andresuribe87 @frankhinek @mistermoe @nitro-neal