HydraCG / Specifications

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

Vocabulary extensions #182

Closed alien-mcl closed 5 years ago

alien-mcl commented 5 years ago

Extended vocabulary with these features:

In general, these imperfect attempt to extend the Hydra Core Vocabulary (HCV) should enable the it for some of the uses cases that appeared on various occasions. Feel free to deliberate more, but I hope these will find their way to the final specification.

I tried to keep backward compatibility so current implementations using the vocabulary in it's current state won't get broken. Unfortunately, there are some minor changes to some of the elements.

I've also added an updated version of the vocabulary.png diagram (including source code).

I'll try to provide a proper PR for Heracles.ts reference client covering these features ASAP.

alien-mcl commented 5 years ago

I've just pushed PR to Heracles.ts with support for these features.

alien-mcl commented 5 years ago

I strongly disapprove of this pull request!

Ok. Indeed this is an imperfect attempt.

looks like there are several unrelated changes

Indeed - there are several topics onboard

none of the proposed changes have a related (or mentioned) PR and have not gone through some analysis beforehand

I partially disagree - operations with explicit targets were discussed quite extensively; other thing it didn't end up with any specific conclusion

it literally came out of the blue

I disagree - most of topics touched here were either discussed or at least mentioned; I also did advertised this attempt in various occasions

Instead of haphazardly submitting PRs we must follow a more structured workflow and follow some ground rules as closely as possible

What kind of structured workflow? I agree that we didn't provide any detailed roadmap for future work, but GH is full of unresolved issues that needs to be addressed and should be treated as that roadmap. I remember how we did work in last several years and look were we are now. I'd like to take a more agile approach with fail-fast attempts. I prefer to provide imperfect but working prototype (look at this PR at Heracles.ts) than to plot and plan and have endless discussions of greatness of Ionian school over Milesian school.

each PR should address a specific issue or at least related issues

I can cherry-pick each implemented feature as a separate PR if that will work for you.

each issue should be first discussed, deemed important and have a rough idea of the solution

As I already said - some of the features were discussed - some of them indeed may need some discussion. This is why that PR came anyway - to start discussion on some real approaches. I don't want to raise new issues to address other issues - it's pointless.

PR with new features should not only introduce vocabulary elements but also extend upon the human-readable part of the spec.

I may agree with this, but this would imply enormous effort on me still having a large risk of a complete failure. Once I confirm that these changes are going in the right direction, I may consider spec changes.

I could go one step further and suggest that the prose should even come slightly before vocabulary. That way it could be easier to understand the intent and gauge the effect of said changes. On the other hand a discussion under the related issue would serve that purpose too.

Also a discussion on some real implementation still serves that purpose. Cherry-picking each feature with some brief would improve how these changes should be understood.

tpluscode commented 5 years ago

Without going into detailed discussions here I think this PR should be split into 3 PRs, which address the three linked issues it aims to resolve. The other two bullets should (non-RDF payloads and headers) should be first discussed as issues.

Splitting into smaller chunks will allow for focused discussions and prevent one change block an unrelated one from being merged.

On a technical note, it's best to avoid submitting PR from downstream master branch because such can cause conflicts once it's merged.

alien-mcl commented 5 years ago

Yep - I've just noticed master branch. One more reason to split it. As for new features - I'll try to dig through GH to find related issues. I'm pretty sure headers were mentioned somewhere.

alien-mcl commented 5 years ago

I've divided it into separate PRs (#183, #184, #185 and #186) - this PR is obsoleted by those.