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

Added contributors field to import schema #392

Merged
merged 7 commits into from
May 24, 2024

Conversation

Billa05
Copy link
Contributor

@Billa05 Billa05 commented Jan 19, 2024

This PR closes #8718
from open library repo.

Description:

Added missing contributors field to import schema

@cdrini

@hornc
Copy link
Collaborator

hornc commented Jan 19, 2024

Thank you @Billa05!

I'll let @cdrini comment too, but I think we could also remove the original contributions string only version from this file at the same time. I don't think any existing import process using this API is sending contributions and I think we should deprecate it. Removing it from the schema so that the one you just added is the only contributors field for imports would avoid any confusion.

Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Looking good! A few changes.

Can you also try validating this? Maybe using a site like https://www.jsonschemavalidator.net/ just to make sure we have all the syntax right! :)

olclient/schemata/import.schema.json Outdated Show resolved Hide resolved
olclient/schemata/import.schema.json Outdated Show resolved Hide resolved
olclient/schemata/import.schema.json Outdated Show resolved Hide resolved
@cdrini
Copy link
Collaborator

cdrini commented Jan 22, 2024

@hornc Let's label it as deprecated I think instead of removing it entirely, and place it at the bottom of the doc. The endpoint does still accept this field, so I think it's useful documentation to note that!

See https://json-schema.org/draft/2019-09/json-schema-validation#rfc.section.9.3

@Billa05 Billa05 force-pushed the update/import_schema branch from 1b9d3cf to deecd2a Compare January 29, 2024 03:54
@Billa05 Billa05 requested a review from cdrini January 29, 2024 04:00
@Billa05 Billa05 requested a review from cdrini February 8, 2024 13:18
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Lgtm! Tested on https://www.jsonschemavalidator.net/ and it worked correctly.

olclient/schemata/import.schema.json Outdated Show resolved Hide resolved
@cdrini cdrini merged commit 7e45d51 into internetarchive:master May 24, 2024
4 checks passed
@Mohammed-Alanazisa
Copy link

This PR closes #8718 from open library repo.

Description:

Added missing contributors field to import schema

@cdrini

Copy link

@CLOUDCUMPUTING CLOUDCUMPUTING left a comment

Choose a reason for hiding this comment

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

DE PAUL

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.

Add missing contributors field to import schema
5 participants