HydraCG / Specifications

Specifications created by the Hydra W3C Community Group
Other
138 stars 26 forks source link

Added new vocabulary elements #157

Closed alien-mcl closed 6 years ago

alien-mcl commented 6 years ago

Added new vocabulary elements as in #41 and #16


This change is Reviewable

lanthaler commented 6 years ago

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


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

Previously, alien-mcl (Karol Szczepański) wrote…
Well, #41 is marked as resolved with `hydra:manages` as one of the solutions. While I think it indeed solves some problems, I've got personal issues with `hydra:collection` as I don't see any reason for making a special distinction between ordinary link and collection link (as this is how I understand `hydra:collection`)

Yep, there was a call for consensus with no objections


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

    "Collection": "hydra:Collection",
    "member": { "@id": "hydra:member", "@type": "@id" },
    "manages": { "@id": "hydra:manages", "@type": "@id" },

We can simplify this to simply "hydra:manages"


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

Allows to link multiple collections to an owning resource.

Please change this to something like "Collections somehow related to this resource."


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

"Provides details on on the resource being a subject in relation described by the collection."

As we might re-use this elsewhere at some point I'd suggest to change it to simply "The subject."


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

      "comment": "Provides details on on the resource being a subject in relation described by the collection.",
      "vs:term_status": "testing"
    },

Looks like you forgot to add hydra:object


Comments from Reviewable

alien-mcl commented 6 years ago

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


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

Previously, lanthaler (Markus Lanthaler) wrote…
We can simplify this to simply `"hydra:manages"`

Are you sure? Indeed when the value of this relation will be a JSON-object, expansion will work. But I've noticed that string literals are not correctly interpreted as resource IRIs. I know, that in most cases there will be an object here, but still, corner case may get broken.


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

Previously, lanthaler (Markus Lanthaler) wrote…
> Allows to link multiple collections to an owning resource. Please change this to something like "Collections somehow related to this resource."

Done.


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

Previously, lanthaler (Markus Lanthaler) wrote…
> "Provides details on on the resource being a subject in relation described by the collection." As we might re-use this elsewhere at some point I'd suggest to change it to simply "The subject."

Done.


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

Previously, lanthaler (Markus Lanthaler) wrote…
Looks like you forgot to add `hydra:object`

Hmm. Honestly - I though it was enlisted mistakenly.

I acknowledged hydra:manages block is to re-instantiate the relation between the resource owning a given collection (i.e. resource being on the left of the hydra:collection predicate) and the resources provided by the collection. Having an explicit hydra:object here could prove difficult for interpretation on what actually that owning resource is doing here in case hydra:object does not point to that owning resource.

Also having all subject-property-object components, why having them repeated in the Hydra vocab instead of using rdf reification ones?

I've added it, as the #41 states it as a solution, but I'm not convinced we need that.


Comments from Reviewable

tpluscode commented 6 years ago

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


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

Previously, alien-mcl (Karol Szczepański) wrote…
Are you sure? Indeed when the value of this relation will be a JSON-object, expansion will work. But I've noticed that string literals are not correctly interpreted as resource IRIs. I know, that in most cases there will be an object here, but still, corner case may get broken.

I agree with Karol. "manages": "some:url" should be interpreted correctly


Comments from Reviewable

tpluscode commented 6 years ago

Reviewed 1 of 1 files at r3. Review status: all files reviewed at latest revision, 4 unresolved discussions.


Comments from Reviewable

lanthaler commented 6 years ago

Review status: all files reviewed at latest revision, 1 unresolved discussion.


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

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
I agree with Karol. `"manages": "some:url"` should be interpreted correctly

I'd fine it very surprising to find URLs as the value of this. So I prefer this to fail in a predictable and easy to spot manner.


Comments from Reviewable

tpluscode commented 6 years ago

Review status: all files reviewed at latest revision, 1 unresolved discussion.


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

Previously, lanthaler (Markus Lanthaler) wrote…
I'd fine it very surprising to find URLs as the value of this. So I prefer this to fail in a predictable and easy to spot manner.

Oh, the object will be the "manages block", right? Yes, it probably makes little sense to have type coercion here


Comments from Reviewable

lanthaler commented 6 years ago

Review status: all files reviewed at latest revision, 1 unresolved discussion.


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

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Oh, the object will be the "manages block", right? Yes, it probably makes little sense to have type coercion here

Yes, the it will be the "manages block"


spec/latest/core/core.jsonld, line 39 at r3 (raw file):

    "manages": { "@id": "hydra:manages", "@type": "@id" },
    "subject": { "@id": "hydra:subject", "@type": "@id" },
    "object": { "@id": "hydra:subject", "@type": "@id" },

hydra:subject

This needs to be hydra:object. Could you please type-coerce both to @vocab (like property)


spec/latest/core/core.jsonld, line 172 at r3 (raw file):

      "@type": "rdf:Property",
      "label": "property",
      "comment": "A property, either supported by a class or described by a collection",

This just makes the documentation more confusing. I'd suggest to revert this change.


Comments from Reviewable

lanthaler commented 6 years ago

Reviewed 1 of 1 files at r3. Review status: all files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

alien-mcl commented 6 years ago

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


spec/latest/core/core.jsonld, line 39 at r3 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
`hydra:subject` This needs to be `hydra:object`. Could you please type-coerce both to `@vocab` (like `property`)

Done.


spec/latest/core/core.jsonld, line 172 at r3 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
This just makes the documentation more confusing. I'd suggest to revert this change.

Done.


Comments from Reviewable

lanthaler commented 6 years ago

Thanks Karol!


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


Comments from Reviewable