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

META: Add validation properties to all fields #102

Open
jpmckinney opened this issue Apr 4, 2019 · 4 comments
Open

META: Add validation properties to all fields #102

jpmckinney opened this issue Apr 4, 2019 · 4 comments
Labels
Core Relates to a recommended extension Schema Involves editing the schema
Milestone

Comments

@jpmckinney
Copy link
Member

jpmckinney commented Apr 4, 2019

See open-contracting/standard#859

This can be done by running ocdskit schema-strict on the schemas.

Note: OCDS Kit's schema-strict will add uniqueItems to the location extension's geometry field, which is not desirable.

@jpmckinney jpmckinney added Schema Involves editing the schema Community Relates to a regular extension labels Apr 4, 2019
@jpmckinney jpmckinney added this to the 1.1.5 milestone Apr 4, 2019
@jpmckinney jpmckinney changed the title Add validation propertie to all fields Add validation properties to all fields Apr 4, 2019
@jpmckinney jpmckinney changed the title Add validation properties to all fields META: Add validation properties to all fields Nov 13, 2019
@romifz romifz self-assigned this Apr 20, 2020
@romifz
Copy link

romifz commented Apr 21, 2020

Hey @jpmckinney, I'm not sure of the scope of this issue.

The title of open-contracting/standard#859 is "All strings, arrays, objects should have minLength, minItems, minProperties" and the first comment says:

If a field is optional, these validation properties are ignored, unless the field is present.

But in open-contracting-extensions/ocds_contract_signatories_extension#21 you say:

We typically don't set minItems on optional fields.

Is this issue about checking that all required fields have proper validation properties, then?

@jpmckinney
Copy link
Member Author

Sorry, I was confusing our current practice with our desired practice :)

Your understanding is correct (from open-contracting/standard#859).

However, instead of making PRs against individual extensions, I think it's better to make a PR to change the schema-strict command in OCDS Kit, to make the changes programmatically. I can then run the updated command against all extensions at once, and commit the changes (I've done this a few times before for other issues).

For a first PR, we can keep the logic simple. There might be cases where we can be more restrictive, based on the specific logic of the field, but we need to be careful like in open-contracting-extensions/ocds_contract_signatories_extension#21, because a rule that works in a compiled release might not work in an individual release.

@romifz
Copy link

romifz commented Apr 22, 2020

Thanks James, that makes sense. I'll check the schema-strict command.

@jpmckinney
Copy link
Member Author

Done for non-core extensions. Leaving open for core extensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Relates to a recommended extension Schema Involves editing the schema
Projects
Archived in project
Development

No branches or pull requests

2 participants