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

Update to JSON:API v1.1 #461

Merged
merged 16 commits into from
Jun 20, 2023
Merged

Conversation

rartino
Copy link
Contributor

@rartino rartino commented Mar 19, 2023

This is a set of changes that I think is what is needed (and useful) for updating the specification to JSON:API v1.1.

The primary reason I push for this is to be optionally allowed to include a top level "@context" to describe an OPTIMADE response as linked data as that is useful for #445. This is allowed in v1.1 but not in v1.0.

I've deprecated (but not removed) the old way of specifying a schema in favor of the more official JSON:API way.

I've consistently updated "JSON API" to "JSON:API" since that seems to be how it is supposed to be written.

I've also added documentation on a couple of OPTIONAL fields that arguably were already allowed from us adhering to JSON:API, but we've discussed is useful as we progress. E.g., the toplevel jsonapi to identify responses to be OPTIMADE responses.

optimade.rst Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
…schema from RECOMMENDED and clarify the replacement field.
optimade.rst Outdated Show resolved Hide resolved
ml-evs
ml-evs previously approved these changes Mar 20, 2023
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, thanks @rartino!

optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
@rartino
Copy link
Contributor Author

rartino commented Jun 9, 2023

Workshop merge this, after fixing the comment above.

optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
@rartino
Copy link
Contributor Author

rartino commented Jun 11, 2023

This is now waiting for (re)reviews.

@rartino rartino requested a review from ml-evs June 11, 2023 18:17
@rartino rartino added the PR/ready-for-review Add this flag if you are the author of the PR and you want it to be reviewed. Remove it when editing label Jun 11, 2023
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
Co-authored-by: Andrius Merkys <[email protected]>
@rartino rartino requested a review from merkys June 13, 2023 00:02
merkys
merkys previously approved these changes Jun 13, 2023
@rartino
Copy link
Contributor Author

rartino commented Jun 13, 2023

Ping @ml-evs

optimade.rst Show resolved Hide resolved
ml-evs
ml-evs previously approved these changes Jun 17, 2023
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rartino!

optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
ml-evs
ml-evs previously approved these changes Jun 17, 2023
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've just rebased this onto the links object PR, and fixed a bit of formatting. Perhaps @rartino can just double-check I haven't broken any content and we can get one more external re-reviews

Copy link
Contributor Author

@rartino rartino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've looked it trough, and it looks good to go.

Edit but looking through the formatted version, I think we are going to need a pre-release 1.2 PR to just clean up some other formatting in general.

@ml-evs
Copy link
Member

ml-evs commented Jun 17, 2023

I've looked it trough, and it looks good to go.

Edit but looking through the formatted version, I think we are going to need a pre-release 1.2 PR to just clean up some other formatting in general.

Sure, we could start collecting them in #475?

optimade.rst Outdated Show resolved Hide resolved
ml-evs
ml-evs previously approved these changes Jun 17, 2023
@ml-evs ml-evs requested review from JPBergsma and merkys June 17, 2023 19:00
merkys
merkys previously approved these changes Jun 20, 2023
optimade.rst Outdated Show resolved Hide resolved
@ml-evs ml-evs dismissed stale reviews from merkys and themself via df194d5 June 20, 2023 08:04
@ml-evs ml-evs merged commit 9645e66 into Materials-Consortia:develop Jun 20, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/ready-for-review Add this flag if you are the author of the PR and you want it to be reviewed. Remove it when editing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants