Closed alien-mcl closed 3 years ago
Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.
spec/latest/core/core.jsonld, line 46 at r1 (raw file):
"next": { "@id": "hydra:next", "@type": "@id" }, "previous": { "@id": "hydra:previous", "@type": "@id" }, "memberTemplate": { "@id": "hydra:memberTemplate", "@type": "@id" },
This can be simplified to just "hydra:memberTemplate"
. Pointing it to a URL would be really confusing.
spec/latest/core/core.jsonld, line 394 at r1 (raw file):
"An IRI template of a collection's member."
Nit: Something like "A IRI template to directly access collection members." might be slightly clearer
Comments from Reviewable
Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.
spec/latest/core/core.jsonld, line 46 at r1 (raw file):
This can be simplified to just `"hydra:memberTemplate"`. Pointing it to a URL would be really confusing.
Done.
spec/latest/core/core.jsonld, line 394 at r1 (raw file):
> "An IRI template of a collection's member." Nit: Something like "A IRI template to directly access collection members." might be slightly clearer
Done.
Comments from Reviewable
Review status: 0 of 2 files reviewed at latest revision, 13 unresolved discussions.
spec/latest/core/core.jsonld, line 392 at r2 (raw file):
rdf:Property
This should be a hydra:TemplatedLink
spec/latest/core/index.html, line 861 at r2 (raw file):
<section> <h3>Collection members' link template</h3>
member's -> member
spec/latest/core/index.html, line 863 at r2 (raw file):
<h3>Collection members' link template</h3> <p>In some circumstances it is useful to give a direct access to collection's
to collection's -> to a collection's
spec/latest/core/index.html, line 864 at r2 (raw file):
<p>In some circumstances it is useful to give a direct access to collection's members by providing an instruction on how to construct an URL by the client.
by the client -> to the client
spec/latest/core/index.html, line 865 at r2 (raw file):
This URL then can be used either to obtain member's details or to update it, i.e. as a result of some batch processing.
--> This URL may then be used to either retrieve a member's details or to update it.
I would drop the "i.e. as a result of some batch processing" as I find it confusing and it doesn't add much.
spec/latest/core/index.html, line 867 at r2 (raw file):
Hydra introduces memberTemplate link that directly binds an owning
Hydra provides the memberTemplate property to describe the URL structure of collection members with an IriTemplate.
spec/latest/core/index.html, line 871 at r2 (raw file):
<pre class="example nohighlight" data-transform="updateExample" title="Collection with a direct access to it's members via an IRI template member link.">
it's --> its
I would also drop the "member link." at the end (including the dot as elsewhere).
spec/latest/core/index.html, line 879 at r2 (raw file):
"totalItems": "329", "member": [ ####... a subset of the members of the Collection ...####
If it is a subset it needs to be a partial collection. I'd just update the comment and keep it a Collection
spec/latest/core/index.html, line 883 at r2 (raw file):
http://api.example/com/users{?userName}
Please use http://api.example.com/users/{userName}
to be consistent with the other examples
spec/latest/core/index.html, line 888 at r2 (raw file):
"@type": "IriTemplateMapping", "variable": "userName", "property": "http://vocab.example.com/userName",
http://vocab.example.com/userName
--> http://api.example.com/vocab#userName
spec/latest/core/index.html, line 894 at r2 (raw file):
"@type": "Operation", "method": "PUT", "expects": "http://vocab.example.com/User"
--> http://api.example.com/vocab#User
spec/latest/core/index.html, line 896 at r2 (raw file):
"expects": "http://vocab.example.com/User" } }
Please indent using spaces as the rest of the document instead of tabs
spec/latest/core/index.html, line 904 at r2 (raw file):
the API exposes a direct access to the collection of users by providing an IRI template that expects a user name property making it a fully functional link. There is also an operation declared that enables client to update a given user with an HTTP <i>PUT</i> method.
the API describes how to directly access a member of the /users
collection through an IRI Template requiring the user name. There is also an operation that tells a client that it can use HTTP PUT on those resources.
Comments from Reviewable
Review status: 0 of 2 files reviewed at latest revision, 13 unresolved discussions.
spec/latest/core/core.jsonld, line 392 at r2 (raw file):
> rdf:Property This should be a `hydra:TemplatedLink`
Done.
spec/latest/core/index.html, line 861 at r2 (raw file):
member's -> member
Done.
spec/latest/core/index.html, line 863 at r2 (raw file):
to collection's -> to a collection's
Done.
spec/latest/core/index.html, line 864 at r2 (raw file):
by the client -> to the client
Done.
spec/latest/core/index.html, line 865 at r2 (raw file):
> This URL then can be used either to obtain member's details or to update it, i.e. as a result of some batch processing. --> This URL may then be used to either retrieve a member's details or to update it. I would drop the "i.e. as a result of some batch processing" as I find it confusing and it doesn't add much.
Done.
spec/latest/core/index.html, line 867 at r2 (raw file):
> Hydra introduces memberTemplate link that directly binds an owning Hydra provides the memberTemplate property to describe the URL structure of collection members with an IriTemplate.
Done.
spec/latest/core/index.html, line 871 at r2 (raw file):
it's --> its I would also drop the "member link." at the end (including the dot as elsewhere).
Done.
spec/latest/core/index.html, line 879 at r2 (raw file):
If it is a subset it needs to be a partial collection. I'd just update the comment and keep it a Collection
Done.
spec/latest/core/index.html, line 883 at r2 (raw file):
> `http://api.example/com/users{?userName}` Please use `http://api.example.com/users/{userName}` to be consistent with the other examples
Done.
spec/latest/core/index.html, line 888 at r2 (raw file):
`http://vocab.example.com/userName` --> `http://api.example.com/vocab#userName`
Done.
spec/latest/core/index.html, line 894 at r2 (raw file):
--> `http://api.example.com/vocab#User`
Done.
spec/latest/core/index.html, line 896 at r2 (raw file):
Please indent using spaces as the rest of the document instead of tabs
Done.
spec/latest/core/index.html, line 904 at r2 (raw file):
> the API exposes a direct access to the collection of
> users by providing an IRI template that expects a user name property making
> it a fully functional link. There is also an operation declared that enables
> client to update a given user with an HTTP PUT method.
the API describes how to directly access a member of the /users
collection through an IRI Template requiring the user name. There is also an operation that tells a client that it can use HTTP PUT on those resources.
Done.
Comments from Reviewable
thanks! I will merge this right after you applied the two remaining changes
Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.
spec/latest/core/index.html, line 867 at r2 (raw file):
Done.
The "the" before memberTemplate is missing
spec/latest/core/index.html, line 896 at r2 (raw file):
Done.
There are still tabs left both below and above
Comments from Reviewable
Hm, I'd actually hold this one off if we're possibly going to actually merge #154 (Actions with explicit target) , which would make memberTemplate
unnecessary
Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.
Comments from Reviewable
Despite taking actions into the spec or not, I think we'll still need something that strengthens connection between the collection and the template so it is not a mere generic action.
Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.
Comments from Reviewable
Isn't that where the action's type comes in? As proposed, schema:DiscoverAction could be a good existing fit. If not we could expand the actions and not keep adding more and more predicates
Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.
Comments from Reviewable
Hm, I'd actually hold this one off if we're possibly going to actually merge #154 (Actions with explicit target)
Good point
Reviewed 2 of 2 files at r3. Review status: all files reviewed at latest revision, 2 unresolved discussions.
Comments from Reviewable
Review status: all files reviewed at latest revision, 2 unresolved discussions.
spec/latest/core/index.html, line 867 at r2 (raw file):
The "the" before memberTemplate is missing
Done.
spec/latest/core/index.html, line 896 at r2 (raw file):
There are still tabs left both below and above
Ehh, IDEs. Maybe we should introduce .editorconfig
file? Done
Comments from Reviewable
Reviewed 1 of 1 files at r4. Review status: all files reviewed at latest revision, all discussions resolved.
Comments from Reviewable
Thanks Karol. This would be ready to merge, but we agreed to hold off till we resolved #154.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.
Comments from Reviewable
This feature has never made it to the spec.
This pull request adds a hydra:memberTemplate discussed in #16 predicate to the vocabulary with a piece of documentation to the spec.
This change is