- From: Karol Szczepański via GitHub <sysbot+gh@w3.org>
- Date: Mon, 30 Oct 2017 06:52:22 +0000
- To: public-hydra-logs@w3.org
Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions. --- *[drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1](https://reviewable.io:443/reviews/hydracg/specifications/143#-KxZXSL5Owpv1cqTG5AR:-Kxfzzr3SmwAGoblziz9:b-79pn0s) ([raw file](https://github.com/hydracg/specifications/blob/f03e3028e6417adf2767a44c0c854b4af3b5fb4d/drafts/use-cases/5.1.creating-new-event-indirectly.md#L34)):* <details><summary><i>Previously, elf-pavlik (elf Pavlik) wrote…</i></summary><blockquote> If client doesn't use anywhere proposed by you `http://example.com/vocab/createEvent` why even bother with introducing it? If you would just like to traverse all links, you could just use `rdfs:seeAlso` and it would give you the same result. IMO if you would like to introduce in the example `http://example.com/vocab/createEvent` client should use it, I would prefer something generic like `hydra:add` (or 'hydra:memberTemplate`) but still client would have to make use of it when finding intended operation. </blockquote></details> I feel like 'hydra:add' would be redundant to 'schema:AddAction', which is the convention I believe we agreed on. Still, we need to link the collection with the IriTemplate'd resource - I think the link's address is irrelevant from the client point of view. I also feel that this `recursive` flag is unneeded - all look-ups should be recursive within current resource. It should also use API documentation obtained at the very beginning. --- *[drafts/use-cases/5.1.creating-new-event-indirectly.md, line 36 at r1](https://reviewable.io:443/reviews/hydracg/specifications/143#-KxZY9gJ8iisuB-N2HfY:-Kxg-hkJK8ER2_GZvyLi:b-o5x9gs) ([raw file](https://github.com/hydracg/specifications/blob/f03e3028e6417adf2767a44c0c854b4af3b5fb4d/drafts/use-cases/5.1.creating-new-event-indirectly.md#L36)):* <details><summary><i>Previously, tpluscode (Tomasz Pluskiewicz) wrote…</i></summary><blockquote> I like that. Only I would make this more functional instead of mutating the operation ``` js const operationTargetExpanded = operation.expandTarget(templateVariables); client.invoke(operationTargetExpanded, event); ``` </blockquote></details> I agree - we shouldn't mutate the original operation. --- *[drafts/use-cases/5.1.creating-new-event-indirectly.md, line 58 at r1](https://reviewable.io:443/reviews/hydracg/specifications/143#-KxY28Opn5nMlgaMrFGV-r1-58:-Kxg-q3PLjEN3HGJtkI6:bukobdj) ([raw file](https://github.com/hydracg/specifications/blob/f03e3028e6417adf2767a44c0c854b4af3b5fb4d/drafts/use-cases/5.1.creating-new-event-indirectly.md#L58)):* <details><summary><i>Previously, lanthaler (Markus Lanthaler) wrote…</i></summary><blockquote> +1.. but to answer the question: `manages` as currently defined only says that all members of the collection in this case are of type Event (nothing more and nothing less). I agree that we should find a better name for this. </blockquote></details> Assuming the semantics given by @lanthaler, let's leave it for now. --- *[drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1](https://reviewable.io:443/reviews/hydracg/specifications/143#-KxYcL7VkgLlvZsRJlA4:-Kxg0LzyohGReGiEUpuD:b-kp4a3i) ([raw file](https://github.com/hydracg/specifications/blob/f03e3028e6417adf2767a44c0c854b4af3b5fb4d/drafts/use-cases/5.1.creating-new-event-indirectly.md#L64)):* <details><summary><i>Previously, elf-pavlik (elf Pavlik) wrote…</i></summary><blockquote> This may work for something like 'Like' a movie etc. where movie would link to a not yet existing resource which once created would represent like by authenticated person (Vimeo and many other APIs style). For a collection to specify a way just add a one single resource to it doesn't seem practical at all. I can't imagine any service doing that since it would require client to re-request the collection every time before adding a new resource to it just to get a new IRI. </blockquote></details> I don't have any specific likes or dislikes for what was written above - just don't let us forget we shouldn't neither forbid nor enforce anything specific. Examples by @tpluscode could happen --- *[drafts/use-cases/5.1.creating-new-event-indirectly.md, line 97 at r1](https://reviewable.io:443/reviews/hydracg/specifications/143#-KxciNXcDOyiU5-EUn3l:-Kxg19zqmIfP_yZHponE:b-wnvho6) ([raw file](https://github.com/hydracg/specifications/blob/f03e3028e6417adf2767a44c0c854b4af3b5fb4d/drafts/use-cases/5.1.creating-new-event-indirectly.md#L97)):* <details><summary><i>Previously, elf-pavlik (elf Pavlik) wrote…</i></summary><blockquote> I think we can make and Issue for that since UC5 also has it and don't block this PR if it doesn't introduce that issue </blockquote></details> I'm not sure what is the problem - what does the JSON-LD spec says about relative URL's? How does it differ from the payload obtained with `GET`? --- *[drafts/use-cases/5.1.creating-new-event-indirectly.md, line 116 at r1](https://reviewable.io:443/reviews/hydracg/specifications/143#-KxcirKWssEpfkdjX8eA:-Kxg2ICOQFj9jCr5p5HL:b-orumjy) ([raw file](https://github.com/hydracg/specifications/blob/f03e3028e6417adf2767a44c0c854b4af3b5fb4d/drafts/use-cases/5.1.creating-new-event-indirectly.md#L116)):* <details><summary><i>Previously, lanthaler (Markus Lanthaler) wrote…</i></summary><blockquote> Not this specification but it would be a violation of the [HTTP specification](https://tools.ietf.org/html/rfc7231#section-4.3.4): > The PUT method requests that the state of the target resource be > created or replaced with the state defined by the representation > enclosed in the request message payload. A successful PUT of a given > representation would suggest that a subsequent GET on that same > target resource will result in an equivalent representation being > sent in a 200 (OK) response. However, there is no guarantee that > such a state change will be observable, since the target resource > might be acted upon by other user agents in parallel, or might be > subject to dynamic processing by the origin server, before any > subsequent GET is received. A successful response only implies that > the user agent's intent was achieved at the time of its processing by > the origin server. > [...] > The fundamental difference between the POST and PUT methods is > highlighted by the different intent for the enclosed representation. > The target resource in a POST request is intended to handle the > enclosed representation according to the resource's own semantics, > whereas the enclosed representation in a PUT request is defined as > replacing the state of the target resource. Hence, the intent of PUT > is idempotent and visible to intermediaries, even though the exact > effect is only known by the origin server. </blockquote></details> Good point --- *Comments from [Reviewable](https://reviewable.io:443/reviews/hydracg/specifications/143)* <!-- Sent from Reviewable.io --> -- GitHub Notification of comment by alien-mcl Please view or discuss this issue at https://github.com/HydraCG/Specifications/pull/143#issuecomment-340357857 using your GitHub account
Received on Monday, 30 October 2017 06:52:25 UTC