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

Added hydra:memberTemplate #158

Closed
wants to merge 6 commits into from

Conversation

alien-mcl
Copy link
Member

@alien-mcl alien-mcl commented Mar 7, 2018

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 Reviewable

@lanthaler
Copy link
Member

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

@alien-mcl
Copy link
Member Author

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.


spec/latest/core/core.jsonld, line 46 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

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):

Previously, lanthaler (Markus Lanthaler) wrote…

"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

@lanthaler
Copy link
Member

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

@alien-mcl
Copy link
Member Author

Review status: 0 of 2 files reviewed at latest revision, 13 unresolved discussions.


spec/latest/core/core.jsonld, line 392 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

rdf:Property

This should be a hydra:TemplatedLink

Done.


spec/latest/core/index.html, line 861 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

member's -> member

Done.


spec/latest/core/index.html, line 863 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

to collection's -> to a collection's

Done.


spec/latest/core/index.html, line 864 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

by the client -> to the client

Done.


spec/latest/core/index.html, line 865 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

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):

Previously, lanthaler (Markus Lanthaler) wrote…

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):

Previously, lanthaler (Markus Lanthaler) wrote…

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):

Previously, lanthaler (Markus Lanthaler) wrote…

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):

Previously, lanthaler (Markus Lanthaler) wrote…

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):

Previously, lanthaler (Markus Lanthaler) wrote…

http://vocab.example.com/userName --> http://api.example.com/vocab#userName

Done.


spec/latest/core/index.html, line 894 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

--> http://api.example.com/vocab#User

Done.


spec/latest/core/index.html, line 896 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

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):

Previously, lanthaler (Markus Lanthaler) wrote…
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.

Done.


Comments from Reviewable

@lanthaler
Copy link
Member

:lgtm: 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):

Previously, alien-mcl (Karol Szczepański) wrote…

Done.

The "the" before memberTemplate is missing


spec/latest/core/index.html, line 896 at r2 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…

Done.

There are still tabs left both below and above


Comments from Reviewable

@tpluscode
Copy link
Contributor

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

@alien-mcl
Copy link
Member Author

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

@tpluscode
Copy link
Contributor

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

@lanthaler
Copy link
Member

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

@alien-mcl
Copy link
Member Author

Review status: all files reviewed at latest revision, 2 unresolved discussions.


spec/latest/core/index.html, line 867 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

The "the" before memberTemplate is missing

Done.


spec/latest/core/index.html, line 896 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

There are still tabs left both below and above

Ehh, IDEs. Maybe we should introduce .editorconfig file? Done


Comments from Reviewable

@lanthaler
Copy link
Member

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@lanthaler
Copy link
Member

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

@alien-mcl
Copy link
Member Author

This feature has never made it to the spec.

@alien-mcl alien-mcl closed this Nov 4, 2020
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.

3 participants