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

Make "id" optional for services and public keys. #66

Merged
merged 7 commits into from
Oct 19, 2019

Conversation

peacekeeper
Copy link
Contributor

@peacekeeper peacekeeper commented Oct 9, 2019

Fixes #47 (see discussion there).


Preview | Diff

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.

Overall, fine with the change... I think we might want to be stronger about suggesting that ids are a good idea.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated
<code>type</code>, and <code>serviceEndpoint</code> properties, and
MAY include additional properties.
Each service endpoint MAY include an <code>id</code> property.
The array of service endpoints MUST NOT contain duplicate entries
Copy link
Member

Choose a reason for hiding this comment

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

Again, should processors throw errors if they find duplicate entries? We should probably define a conformance class here for a DID Document processor and restate this as "A DID Document Processor MUST throw an error if there are duplicate entries with the same id." ... or something to that effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'd say throw an error in this case, as above. Regarding a "DID Document Processor" conformance class, do you want to do that here in this PR, or would that first have to be introduced/defined elsewhere?

Copy link
Member

@msporny msporny Oct 17, 2019

Choose a reason for hiding this comment

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

Regarding a "DID Document Processor" conformance class, do you want to do that here in this PR, or would that first have to be introduced/defined elsewhere?

Ideally, we'd define a DID Document Processor conformance class first, merge that in, then fix this one and merge it in.

Alternatively, and I think this would be faster, just raise an issue stating that we need to define a DID Document Processor conformance class, fix this PR so it throws an error, we'll merge this, and get to the DID Document Processor conformance class later. Better to get this PR merged than have it hang out there for too long.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added text about producing errors in bb57e32. Also raised issue #80.


<li>
Each service endpoint MUST include <code>type</code> and
<code>serviceEndpoint</code> properties, and MAY include additional
Copy link
Member

Choose a reason for hiding this comment

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

We need to go through the spec and talk about property-value pairs. properties may be confusing to readers... because they're typically the "key" part of a "key-value" pair.

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's an RDF term, right? And it's used in many places throughout the spec. So how about we don't address it in this specific PR, but instead raise a new (editorial) issue to see if we can find a better term.

Copy link
Member

Choose a reason for hiding this comment

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

Not RDF... that's predicate-object (which will just confuse everyone, so we shouldn't use it). "key-value" is concerning because of the prevalence of "public keys" in the spec. We try to define things using JSON-ish terminology, but we should probably avoid the phrase "key-value" and prefer "property-value" to refer to the data model concept of an associative array... but your suggestion works for me, let's deal with it in another PR.

peacekeeper and others added 2 commits October 15, 2019 10:38
Co-Authored-By: Manu Sporny <msporny@digitalbazaar.com>
Co-Authored-By: Manu Sporny <msporny@digitalbazaar.com>
@selfissued
Copy link
Contributor

The JSON terminology is "member": "value".

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
peacekeeper and others added 2 commits October 15, 2019 21:02
Co-Authored-By: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-Authored-By: Manu Sporny <msporny@digitalbazaar.com>
@msporny
Copy link
Member

msporny commented Oct 17, 2019

@selfissued wrote:

The JSON terminology is "member": "value".

Ugh, you're right... I was thinking of Javascript's object properties -- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getOwnPropertyNames

Wonder why JSON deviated from that language...

@peacekeeper
Copy link
Contributor Author

If this is merged, I think it also makes #46 obsolete (see discussion there).

@msporny
Copy link
Member

msporny commented Oct 19, 2019

Discussed during telecon, multiple positive reviews, suggestions applied, no objections, merging.

@msporny msporny merged commit 99a0873 into master Oct 19, 2019
@msporny msporny deleted the peacekeeper-key-and-service-id-optional branch January 7, 2020 15:55
msporny pushed a commit to w3c/controller-document that referenced this pull request Jun 2, 2024
* Make "id" optional for services and public keys.
* Change MAY to SHOULD for public keys.
* Change MAY to SHOULD for service endpoints.
* Say "multiple" instead of "duplicate" entries.
* Apply suggestion from @msporny.
* Add comment about producing an error.
* Address feedback at w3c/did-core#66 (comment).
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.

Should "id" be mandatory for "service" and "publicKey"?
4 participants