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

Clarify that full linkage graph must be rooted in primary data #1541

Conversation

jelhan
Copy link
Contributor

@jelhan jelhan commented Feb 20, 2021

The current wording in specification does not enforce that a graph of related resources in a Compound Document is rooted in primary data. The requirement could be also interpreted as if two included resources referencing each other fulfill it. This has been very well explained by @beauby in #1035.

The original pull request created by @beauby more than four years ago got stale. But the issue is still the same in current specification.

I updated the @beauby's proposal given in #1035 to target v1.1. I also tried to slightly improve the wording.

Closes #1035

@gabesullice
Copy link
Contributor

gabesullice commented Apr 15, 2021

Thanks for reviving this issue @beauby. What do you think about removing the note altogether and simplifying the paragraph to:

Every included resource object MUST be identified via a chain of
relationships originating in a document's primary data. This means that
compound documents require "full linkage" and that no resource object can be
included without a direct or indirect relationship to the document's primary
data.

@jelhan
Copy link
Contributor Author

jelhan commented Apr 16, 2021

I like it. It seems to be more precise than the current version while still being easier to understand. I think you managed to achieve both at the same time. 👏

@dgeb
Copy link
Member

dgeb commented Apr 16, 2021

I agree that @gabesullice's version is a nice improvement. 💯

@jelhan please feel free to update this PR with Gabe's wording or, if you prefer, close it so that he can create a separate PR.

@jelhan
Copy link
Contributor Author

jelhan commented Apr 16, 2021

@jelhan please feel free to update this PR with Gabe's wording

Update the PR. Let me know if I should stash the commits or if you do it anyways as part of merging the pull request.

@dgeb dgeb merged commit f7dadf5 into json-api:gh-pages Apr 16, 2021
@dgeb
Copy link
Member

dgeb commented Apr 16, 2021

Update the PR. Let me know if I should stash the commits or if you do it anyways as part of merging the pull request.

I just merged + squashed to keep it simple.

Thanks @jelhan and @gabesullice!

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.

None yet

4 participants