Skip to content
This repository has been archived by the owner on Oct 29, 2019. It is now read-only.

Add did-query (addresses #85) #106

Closed
wants to merge 1 commit into from
Closed

Conversation

mikelodder7
Copy link
Contributor

@mikelodder7 mikelodder7 commented Oct 10, 2018

Signed-off-by: Michael Lodder redmike7@gmail.com

Addresses issue 85


Preview | Diff

Signed-off-by: Michael Lodder <redmike7@gmail.com>
Copy link
Member

@peacekeeper peacekeeper left a comment

Choose a reason for hiding this comment

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

This is good, yes let's finally add support for the missing did-query. But some details should be changed I think regarding query+fragment, see inline comments.

(i.e., it does NOT contain a path or fragment),
then the DID query operates directly against the DID document.</p>

<p>If a DID is followed by a DID fragment followed immediately by a DID query component,
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph doesn't make sense to me. did-fragment comes after did-query.

then the DID query operates against the branch of the DID document
specified DID document fragment.</p>

<p>A DID method specification MAY defined its own DID fragment query syntax.</p>
Copy link
Member

Choose a reason for hiding this comment

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

What's the motivation here?

A generic <a>DID query</a> (the did-query rule in Section <a href="#the-generic-did-scheme"></a>) is identical to a URI search and MUST
conform to the ABNF of the query rule in [[RFC3986]].
<p>If a DID is followed immediately by a DID query component
(i.e., it does NOT contain a path or fragment),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(i.e., it does NOT contain a path or fragment),
(i.e., it does NOT contain a path),

Choose a reason for hiding this comment

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

I think the point is that the query operations on what the path resolves too. Currently if there is no path the default is the path resolves to the DDO likewise if there is not path then the fragment resolves against the DDO. The query applies to what ever the path resolves too. The query usually does not apply to a fragment but to the resource. However because the did fragment has special semantics we could give special meaning to the query if a did fragment is present. I like the change does not contain a path and let the implementer decide what to do if a did fragment is present but not path.

@rhiaro rhiaro added discuss Wider discussion in an issue or meeting required before merging elsewhere Belongs on a different spec needs new pr Concept is good but a clean PR is needed eg. to remove conflicts labels Jan 22, 2019
@peacekeeper peacekeeper mentioned this pull request Jan 30, 2019
@davidlehn
Copy link
Contributor

Would be good to rewrite the commit message to say what patch does vs just saying it addresses an issue number.

@rhiaro rhiaro changed the title Address Issue 85 Add did-query (addresses #85) Feb 5, 2019
@dmitrizagidulin
Copy link

Addressed/refactored in PR #168.

@peacekeeper
Copy link
Member

Replaced by PR #189.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discuss Wider discussion in an issue or meeting required before merging elsewhere Belongs on a different spec needs new pr Concept is good but a clean PR is needed eg. to remove conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants