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

Propose version to be a required property for a reviewed preprint #321

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

giorgiosironi
Copy link
Contributor

@giorgiosironi giorgiosironi commented Oct 25, 2024

We have been working on homepage listings which displays some teasers of reviewed preprints.

We have noticed that the schema allows for a reviewed-preprint to not have a version, however we have not found examples in the production data by browsing the website and a couple of pages of the API.

We were wondering if this could be made mandatory, unless there are cases where a reviewed-preprint is allowed not to have a version. The behavior of journal if this case happens is benign (does not explode) but otherwise undefined.

Prerequites

  • include version in all samples in api-dummy
  • include version in all samples in journal
  • make version mandatory in api-sdk-php
  • update samples in this PR

@giorgiosironi giorgiosironi requested a review from nlisgo October 25, 2024 13:02
Copy link
Member

@nlisgo nlisgo left a comment

Choose a reason for hiding this comment

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

Creating it as optional allowed the schema to be rolloed out without it already being implemented. I support a change to make it required.

You will have to amend the samples which is probably why tests are failing.

You would need to ensure that all examples in api-dummy include in order to bump the version. Then on to the adjustment in sdk and then journal any mocks will have to include the version in the mock response.

Happy with this change. If you don't bump the version there is no need for EPP to work on the api we supply.

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.

2 participants