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

Fix: #38. Stubs Conformance. #39

Merged
merged 5 commits into from Jul 6, 2022
Merged

Fix: #38. Stubs Conformance. #39

merged 5 commits into from Jul 6, 2022

Conversation

ioggstream
Copy link
Contributor

@ioggstream ioggstream commented Jul 3, 2022

This PR

  • stubs conformance

Preview | Diff

@ioggstream ioggstream requested a review from gkellogg July 3, 2022 22:15
@ioggstream ioggstream changed the title Fix: #38. Add Conformance. Fix: #38. Stubs Conformance. Jul 3, 2022
Copy link
Member

@gkellogg gkellogg left a comment

Choose a reason for hiding this comment

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

Might want to make actual definition references using data-cite for these terms. The JSON-LD spec created a "Definitions" section and defined the imported terms locally, which were themselves references. Might want to do something similar for consistency.

E.g.:

  <dt><dfn data-cite="JSON-LD11#dfn-json-ld-document">JSON-LD document</dfn></dt><dd>
    A <a>JSON-LD document</a> is a serialization of
    an <a>RDF dataset</a>.
    See the <a data-cite="JSON-LD11#json-ld-grammar">JSON-LD Grammar</a> section in JSON-LD 1.1 for a formal description.
  </dd>

These are defined in https://github.com/w3c/json-ld-wg/blob/main/common/terms.html and used with some clever code which tries to only define those terms actually used. I'm not proposing that we do that here, or that we use the local definition for for imported terms, necessarily, but structure them similarly.

@ioggstream
Copy link
Contributor Author

I suggest to do this referencing job in a subsequent editorial PR once the referenced terms are consolidated
filing another issue like #40

JSON-LD ... defined the imported terms locally

IIUC, in json-ld there are copy-pasted definitions from other documents. Is that correct?
I think we should just use references, so that people is encouraged to read the original documents:
an implementer is expected to read the JSON, JSON-LD and YAML spec.
The fact that they won't do it should be addressed e.g. in the Appendix.

In my recent ramblings on other specs I spent a good amount of time replacing copied definitions with hrefs to make them coherent with the latest HTTP spec (RFC9110): this work would have been easier if they just used hrefs :)

WDYT?

@gkellogg
Copy link
Member

gkellogg commented Jul 4, 2022

IIUC, in json-ld there are copy-pasted definitions from other documents. Is that correct? I think we should just use references, so that people is encouraged to read the original documents: an implementer is expected to read the JSON, JSON-LD and YAML spec. The fact that they won't do it should be addressed e.g. in the Appendix.

Not exactly, consider the term "JSON object". It has an entry in the dl for Terms imported from Other Specifications, but the key is a reference to RFC8259. But, by defining a dfn for this, we can simply use <a>JSON Object</a> rather than the more lengthly <a data-cite="RFC8259#section-4">JSON Object</a> within the body of the document for every occurrence; it will use the reference directly into RFC8259, rather than that in the dl. It simplifies later usage, and provides a single place for us to describe the terms we are importing, vs defining. Yes, there is a brief description, but that is mostly to describe the context for usage within JSON-LD, not to repeat the definition from the referenced spec.

@ioggstream
Copy link
Contributor Author

ioggstream commented Jul 5, 2022

brief description ...

@gkellogg ah, ok. I'll file it in a separate, editorial, issue. Since I'm working on 4 other I-Ds, it would be great if during today's call we could find someone volounteering on those editorial stuff.

See #47

@ioggstream
Copy link
Contributor Author

Moved the other issue to #47. @gkellogg feel free to merge

@ioggstream ioggstream requested a review from gkellogg July 5, 2022 12:45
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
ioggstream and others added 2 commits July 5, 2022 19:27
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
@ioggstream ioggstream added this to the -00 milestone Jul 6, 2022
@@ -262,7 +262,7 @@ <h2>Introduction</h2>
The terms YAML, "YAML document", "YAML representation graph",
Copy link
Contributor

Choose a reason for hiding this comment

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

"YAML document" has been carefully edited out of most of this document, so perhaps it should be removed here as well?

Suggested change
The terms YAML, "YAML document", "YAML representation graph",
The terms YAML, "YAML representation graph",

Copy link
Member

Choose a reason for hiding this comment

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

@ioggstream I'll hold off on merging this until you've responded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original text is ok to me. Let me just rebase the PR @gkellogg :)

Copy link
Member

Choose a reason for hiding this comment

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

PR is merged, thanks.

@gkellogg
Copy link
Member

gkellogg commented Jul 6, 2022

This PR was discussed in the 2022-07-06 meeting, and it was agreed to merge.

@gkellogg gkellogg merged commit fb3dba5 into main Jul 6, 2022
@gkellogg gkellogg deleted the ioggstream-38 branch July 6, 2022 21:24
@github-pages github-pages bot temporarily deployed to github-pages July 6, 2022 21:25 Inactive
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.

None yet

5 participants