Rework Data Model section. #455

msporny · 2020-11-08T21:35:33Z

This PR implements a number of resolutions made during the last DID WG virtual face-to-face meeting. Namely, it adds different classes of properties, defines all datatypes, and moves representation authoring rules to later in document.

Reviewers should look at the rewritten Data Model section here:

https://pr-preview.s3.amazonaws.com/w3c/did-core/pull/455.html#data-model

and the representation authoring guidelines that was moved here:

https://pr-preview.s3.amazonaws.com/w3c/did-core/pull/455.html#creating-representations

Preview | Diff

Add different classes of properties, all datatypes, move representation authoring rules to later in document.

index.html

iherman · 2020-11-09T12:23:43Z

Would it be possible to merge this branch into #454 (or merge #454 into this one)? This PR misses the rules on representation specific properties (ie, resolution 5 and resolution 6) while those are the focus of #454. These are the two sides of the same coin, and it is a bit difficult to comment on each of them separately...

OR13 · 2020-11-10T16:09:13Z

index.html

+      </ol>
+      <p>
+In order to maximize interoperability, representation specification authors
+SHOULD register their representation in the [[?DID-SPEC-REGISTRIES]].


I think we need to provide an "example" of how to do this. w3c/did-extensions#154

I agree, but think that would be best handled in another PR.

Agreed. Also note that the "contents" of those specifications have 3 examples in DID Core: JSON, JSON-LD, and CBOR.

brentzundel

Some minor editorial suggestions, otherwise LGTM.

brentzundel · 2020-11-11T00:09:54Z

index.html

+different classes of properties. The first class of properties are called
+core properties, and are specified in section <a href="#core-properties"></a>.
+The second class of properties are called representation properties, and
+are specified in section <a href="#representations"></a>.


I like the two buckets. I'm not sure if we want to bikeshed the names here, or whether we should worry about that after the primary changes in the spec (i.e., after this PR is merged).

I expect we'll bike shed the names in another PR. This PR assumes that we'll do that later.

index.html

brentzundel · 2020-11-11T00:14:51Z

index.html

+      </ol>
+      <p>
+In order to maximize interoperability, representation specification authors
+SHOULD register their representation in the [[?DID-SPEC-REGISTRIES]].


I agree, but think that would be best handled in another PR.

Co-authored-by: Brent Zundel <[email protected]>

msporny · 2020-11-11T21:17:13Z

@iherman wrote:

Would it be possible to merge this branch into #454 (or merge #454 into this one)?

I was trying to keep the PR relatively small and focused, dealing w/ the base set of changes to the data model per the virtual face to face in this PR, and then @dlongley adding on top of it to address other resolutions made (that he was asked to write a PR for).

Now that we have multiple approvals for this PR, I'm hesitant to do that @iherman because it could affect the review results. I know it's a pain to review, but I'd rather keep the two separate given that we have multiple positive reviews with changes made based on those reviews. We'll have to deal with #454 as a separate PR.

kdenhartog · 2020-11-12T01:59:31Z

@iherman wrote:

Would it be possible to merge this branch into #454 (or merge #454 into this one)?

I was trying to keep the PR relatively small and focused, dealing w/ the base set of changes to the data model per the virtual face to face in this PR, and then @dlongley adding on top of it to address other resolutions made (that he was asked to write a PR for).

Now that we have multiple approvals for this PR, I'm hesitant to do that @iherman because it could affect the review results. I know it's a pain to review, but I'd rather keep the two separate given that we have multiple positive reviews with changes made based on those reviews. We'll have to deal with #454 as a separate PR.

It appears all approvals from Orie, Brent, myself, and Dave (author of #454 and approver of this PR) on this PR have also approved the other PR if it makes it easier to get both through with fewer merge conflicts. Hard to say what other reviewers will view as controversial though and so the age old wisdom of keeping PRs small and focused would lean me towards keeping separate as well.

talltree · 2020-11-12T06:12:27Z

index.html

@@ -2250,6 +2329,36 @@ <h1>Core Representations</h1>
 for more information.
    </p>

+    <section>
+      <h2>Creating Representations</h2>


Suggest changing this heading to "Representation Requirements" but it's okay to do that in a subsequent PR as well.

talltree

Major improvement. Let's pull in ASAP. Can be tweaked by subsequent PRs.

peacekeeper

For the record, I continue to believe that it's a mistake to include representation-specific syntax in the abstract data model and other representations (e.g. to see @context in a YAML file). But I agree this PR reflects the decisions recently made during the TPAC DID WG F2F.

jricher · 2020-11-16T21:00:02Z

index.html

+      <ol>
+        <li>
+A representation MUST define an unambiguous encoding and decoding for all
+property names and their associated values as defined in this specification.


This should be defined in terms of the types associated with the property values and not the values themselves. It's a subtle difference, but it allows us to continue to talk about things in terms of property definitions.

Agree. I just moved the text in this PR and expect that we'll make the change you suggest in a subsequent PR, when the "Representation Requirements" section is updated with rewording / new rules that we might have missed (like how you register a representation in the DID Spec registries).

index.html

jricher · 2020-11-16T21:01:12Z

index.html

+are conformant with the fragment processing rules defined in section
+<a href="#fragment"></a> of this specification.
+        </li>
+      </ol>


The representation must also define default generic type processing rules for consumption of all data types within the representation.

I'm not understanding how this differs from a representation defining unambiguous encoding and decoding for all property names and the data types of their associated values, could you clarify?

I think I understand what @jricher is getting at, and that's planned for a future PR. Basically, each representation must list all the abstract data model data types and how you express those concretely in the representation. The representation sections kinda/sorta have this right now, but if this PR goes in, we'll be able to be far more explicit and prescriptive about those serializations in each representation.

@msporny has the right of it, but it also needs to clearly go both ways, to and from the data model data types and the representation's data types. So you need things like this strawman:

"If you see an Integer in the ADM it goes in a JSON Number"

As well as:

"If you see a JSON Number in the representation, and don't have an additional property definition, it is an ADM Float".

I'm ok with this being in a separate PR but this can't be lost.

brentzundel · 2020-11-17T11:16:08Z

index.html

+A representation MUST define an unambiguous encoding and decoding for all
+property names and their associated values as defined in this specification.
+This enables anything that can be represented in the <a>DID document</a> data
+model to also be represented in a compliant representation.


@jricher would wording this address your concern?

Suggested change

A representation MUST define an unambiguous encoding and decoding for all

property names and their associated values as defined in this specification.

This enables anything that can be represented in the <a>DID document</a> data

model to also be represented in a compliant representation.

A representation MUST define an unambiguous encoding and decoding for all

property names and the data types of their associated values, as defined in

this specification. This enables anything that can be represented in the

<a>DID document</a> data model to also be representable in a compliant

representation.

index.html

Co-authored-by: Justin Richer <[email protected]>

index.html

Co-authored-by: Justin Richer <[email protected]>

msporny · 2020-11-17T23:23:18Z

Normative, multiple reviews, changes requested and made, no objections, RESOLUTION to merge, merging.

iherman · 2020-11-25T12:16:38Z

The issue was discussed in a meeting on 2020-11-24)

no resolutions were taken

View the transcript

3. PR 454 and 455...

See github pull request #454, #455.

Brent Zundel: status update?

dlongely: we are waiting for a response from mike jones regarding the suggestions for adding a note to the PR / regardign @context in the JSON representation section.
… unsure if he is still objecting

Brent Zundel: I will ping him again

Markus Sabadello: after merging 455 and then 454 we still need to revisit the exact terminology... lots of different properties type to hammer down.

See github issue #463.

Markus Sabadello: feedback welcome

Brent Zundel: other PRs that need to be discussed in this meeting?

Rework Data Model section.

e56960b

Add different classes of properties, all datatypes, move representation authoring rules to later in document.

msporny requested review from talltree, dlongley, OR13 and peacekeeper November 8, 2020 21:38

dlongley reviewed Nov 8, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

index.html Outdated Show resolved Hide resolved

msporny added 3 commits November 8, 2020 19:36

Replace decimal with integer. Specify IEEE 754-2008 for double.

5b99168

Fix IEEE 754-2008 reference.

916d94a

Fix number warning in data types.

7234648

dlongley mentioned this pull request Nov 9, 2020

Add representation language to address syntax and "unknown" properties. #454

Merged

kdenhartog approved these changes Nov 10, 2020

View reviewed changes

OR13 reviewed Nov 10, 2020

View reviewed changes

OR13 approved these changes Nov 10, 2020

View reviewed changes

OR13 mentioned this pull request Nov 10, 2020

Data Model and Representations "Value and Lexical Space" #456

Closed

brentzundel approved these changes Nov 11, 2020

View reviewed changes

Apply suggestions to Data Model from @brentzundel code review.

e58f590

Co-authored-by: Brent Zundel <[email protected]>

talltree reviewed Nov 12, 2020

View reviewed changes

talltree approved these changes Nov 12, 2020

View reviewed changes

peacekeeper approved these changes Nov 12, 2020

View reviewed changes

rhiaro approved these changes Nov 16, 2020

View reviewed changes

This was referenced Nov 16, 2020

Define the JSON representation as JSON (without excluding directives) #396

Closed

Normative statements review #384

Closed

Define serializations for Numbers, Dates, other properties #361

Closed

jricher suggested changes Nov 16, 2020

View reviewed changes

brentzundel reviewed Nov 17, 2020

View reviewed changes

msporny commented Nov 17, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

Rename "Creating Representations" to "Representation Requirements".

383e627

Ensure representations associated with one MIME type.

a656a88

Co-authored-by: Justin Richer <[email protected]>

msporny commented Nov 17, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

Make language around data type requirements more specific.

ecda543

Co-authored-by: Justin Richer <[email protected]>

brentzundel approved these changes Nov 17, 2020

View reviewed changes

jricher mentioned this pull request Nov 17, 2020

Representations need explicit type conversion rules #462

Closed

msporny merged commit ca8d10e into master Nov 17, 2020

msporny mentioned this pull request Nov 17, 2020

updated cbor section #420

Merged

This was referenced Nov 18, 2020

Naming classes of properties and syntax #463

Closed

Should CDDL replace INFRA as the language of the ADM is a question that needs to be resolved by the working group? #461

Closed

msporny deleted the msporny-data-model branch November 24, 2020 15:04

msporny mentioned this pull request Nov 24, 2020

Add concrete serialization rules for JSON based on new data types section. #469

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework Data Model section. #455

Rework Data Model section. #455

msporny commented Nov 8, 2020 •

edited by pr-preview bot

Loading

iherman commented Nov 9, 2020

OR13 Nov 10, 2020

brentzundel Nov 11, 2020

msporny Nov 11, 2020

brentzundel left a comment

brentzundel Nov 11, 2020

msporny Nov 11, 2020

brentzundel Nov 11, 2020

msporny commented Nov 11, 2020 •

edited

Loading

kdenhartog commented Nov 12, 2020

talltree Nov 12, 2020

msporny Nov 17, 2020

talltree left a comment

peacekeeper left a comment

jricher Nov 16, 2020

msporny Nov 17, 2020

jricher Nov 16, 2020

brentzundel Nov 17, 2020

msporny Nov 17, 2020

jricher Nov 17, 2020

brentzundel Nov 17, 2020 •

edited

Loading

msporny commented Nov 17, 2020

iherman commented Nov 25, 2020

3. PR 454 and 455...

Rework Data Model section. #455

Rework Data Model section. #455

Conversation

msporny commented Nov 8, 2020 • edited by pr-preview bot Loading

iherman commented Nov 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brentzundel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msporny commented Nov 11, 2020 • edited Loading

kdenhartog commented Nov 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

talltree left a comment

Choose a reason for hiding this comment

peacekeeper left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brentzundel Nov 17, 2020 • edited Loading

Choose a reason for hiding this comment

msporny commented Nov 17, 2020

iherman commented Nov 25, 2020

3. PR 454 and 455...

msporny commented Nov 8, 2020 •

edited by pr-preview bot

Loading

msporny commented Nov 11, 2020 •

edited

Loading

brentzundel Nov 17, 2020 •

edited

Loading