Skip to content

Rework collection example for clarity #490

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

Merged
merged 6 commits into from
Nov 16, 2017

Conversation

handrews
Copy link
Contributor

Addresses #478.

The first commit is a few trivial typo fixes outside of the collection example.

It's best not to review the 2nd commit directly given the extent of the reworking.
Just build it and read it as an entire new example. It was not reworked in anything
remotely resembling discrete steps, and I wouldn't even know where to start to split it.

@dlax note that several of the output URIs in the main example were flat-out wrong.
As we saw with the older tree example before the big rewrite, if you can't understand
an example it's quite likely not just badly organized but wrong. I'll have to remember
to consider that more closely. This is part of the problem of a spec with no implementation;
I can't just test it out.

Anyway, here are the main changes:

  • Restate the item resource, including the newly added "collection"
    link.
  • Remove some of the more complex discussions that weren't
    needed to introduce the key patterns and challenges.
  • Make pagination and first-item-creation subsections, and put
    pagination first.
  • Write creation guidance as if it generally works, and leave the
    doubts to the crefs.
  • Generally improve the wording and flow. Hopefully.

Note the references to issue #421, which I have to update with some of the things
I figured out while doing this.

* Restate the item resource, including the newly added "collection"
  link.
* Remove some of the more complex discussions that weren't
  needed to introduce the key patterns and challenges.
* Make pagination and first-item-creation subsections, and put
  pagination first.
* Write creation guidance as if it generally works, and leave the
  doubts to the crefs.
* Generally improve the wording and flow.  Hopefully.
Copy link
Member

@dlax dlax left a comment

Choose a reason for hiding this comment

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

It's a clear improvement, thanks for working on this!

are introduced.
</cref>
There are two "item" links, one for each item in the "elements" array.
Unilke the "self" links, these are defined only in the collection schema.
Copy link
Member

Choose a reason for hiding this comment

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

typo: unilke

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I swear I ran the spell checker! Repeatedly! Argh... I need to find an intelligent spell-checker for XML in vim that I can just leave on instead of the convoluted filetype switching that I do to get the regular one to work at all.

</cref>
</t>
<t>
Let's add a link for this collection to the entry point schema, including
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth adding a reference to the "entry point schema" which is currently defined in section 9.1 (we are in section 9.5.1).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call.

@dlax
Copy link
Member

dlax commented Nov 16, 2017

@handrews See also handrews#4 spotted while reading again these examples. (Now that they are clearer, I pay more attention to details!)

handrews and others added 2 commits November 16, 2017 08:34
replace href* keywords by template* in collections example
More review feedback on the collection example.
@handrews
Copy link
Contributor Author

I'm going to go ahead and merge this b/c of the approval plus I accepted the suggested PR, and the remaining fixes are trivial and build passed. And more importantly, anyone reading the pre-publication review materials has no doubt been confused.

@handrews handrews merged commit 5b86e14 into json-schema-org:master Nov 16, 2017
@handrews handrews deleted the collect3 branch November 16, 2017 20:21
@gregsdennis gregsdennis added clarification Items that need to be clarified in the specification and removed Type: Maintenance labels Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Items that need to be clarified in the specification Priority: High
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants