Closed tsachiherman closed 9 months ago
Attention: 20 lines
in your changes are missing coverage. Please review.
Comparison is base (
749f7c1
) 91.79% compared to head (a94ecaf
) 91.25%.
Files | Patch % | Lines |
---|---|---|
src/lib.rs | 0.00% | 13 Missing :warning: |
src/resolver.rs | 94.94% | 5 Missing :warning: |
src/types/ethr.rs | 60.00% | 2 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Looks good to me. One note about process, you should be able to directly create branches in the xmtp/didethresolver repo and PR from there. Let me know if GitHub permissions forced you to fork the repo. Your GitHub user may not be in the right group. IT should be able to fix that.
That was done very much on purpose, so that we won't "spam" the public repository with intermediate branches. Creating shared branches are good practice when we have multiple individuals contributing to the same branch, but aren't really designed for scalability. ( i.e. it would add the role of "cleanup officer" ).
That's the reason why most large organizations that promotes external contributions would allow PR to be created only from a fork.
What ?
This PR add the support for version_id as part of the resolver implementation.
More details
The DID document specifications is described https://github.com/decentralized-identity/ethr-did-resolver/blob/master/doc/did-method-spec.md. The changes here convert the return type from
DidDocument
into aDidResolutionResult
. The difference is this:So that a
DidResolutionResult
provides additional information regarding the document. In particular, it provides versioning information.To take advantage of that, the resolver also support requesting a particular version. That version can be provided as the second argument :
result by