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

conformance_and_extensions.md:update extension guidance #1715

Open
wants to merge 4 commits into
base: 1.2-dev
Choose a base branch
from

Conversation

odscjen
Copy link
Contributor

@odscjen odscjen commented Nov 7, 2024

addresses #582 (not sure if it will close it as an additional PR to ensure this is consistent with the style guide may be needed)

docs/schema/conformance_and_extensions.md Outdated Show resolved Hide resolved
Comment on lines 30 to 55
### Core fields

* Extensions *should not* delete fields from the core schema
* Extensions *should not* change the properties of fields from the core schema. If an extension desires to document further usage of a core standard field, it should do so through documentation, rather than changing the field's `description` property.

### Structure

* All definitions and properties *must* set a `title`, `description` and `type`, unless they are originally defined in the core schema or in another extension in which case they *must* set a `$ref` to the existing object
* If a field's `type` is "array", `items` *must* be set
* If using `items`, its `type` *must* only include "array", "number" and/or "string"
* If an array field's `wholeListMerge` and `omitWhenMerged` properties are not used or are set to `false`, the object fields under it *must* have an `id` field and this `id` field *must* be required

### Codelists

* If `openCodelist` is `true`, `enum` *must not* be set
* If `openCodelist` is `false`, `enum` *must* be set
* If a field has an `enum`, this *must* be expressed as a csv codelist
* If a field has an `enum`, `codelist` and `openCodelist` *must* be set
* If adding codes to an existing codelist the codelist filename *must* append `+` to the start of the core codelist filename, for example `+documentType.csv`

### Field and code names

* Definition names *must* be UpperCamelCase
* Field names *must* be lowerCamelCase
* Definition and field names *must* contain only ASCII alphabetical letters
* If an acronym is used within a field or definition name, the acronym *should* be all UPPERCASE, unless it is at the beginning of the name, in which case it *should* be all lowercase.
Copy link
Contributor

Choose a reason for hiding this comment

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

I've taken a pass at rewording to use consistent imperative form and for strict use of the terms 'keyword', 'property' and 'definition' from JSON Schema. The previous draft used 'properties' to mean both properties and keywords in JSON Schema, and often referred to 'fields' and 'definitions', which mixes OCDS terminology with JSON Schema terminology, e.g. tender.tenderPeriod.startDate is a field in OCDS, but in JSON Schema tender, tenderPeriod and startDate are properties, and tender and tenderPeriod reference the Tender and Period definitions, respectively.

The second bullet point might need to be further refined depending on #582 (comment)

Suggested change
### Core fields
* Extensions *should not* delete fields from the core schema
* Extensions *should not* change the properties of fields from the core schema. If an extension desires to document further usage of a core standard field, it should do so through documentation, rather than changing the field's `description` property.
### Structure
* All definitions and properties *must* set a `title`, `description` and `type`, unless they are originally defined in the core schema or in another extension in which case they *must* set a `$ref` to the existing object
* If a field's `type` is "array", `items` *must* be set
* If using `items`, its `type` *must* only include "array", "number" and/or "string"
* If an array field's `wholeListMerge` and `omitWhenMerged` properties are not used or are set to `false`, the object fields under it *must* have an `id` field and this `id` field *must* be required
### Codelists
* If `openCodelist` is `true`, `enum` *must not* be set
* If `openCodelist` is `false`, `enum` *must* be set
* If a field has an `enum`, this *must* be expressed as a csv codelist
* If a field has an `enum`, `codelist` and `openCodelist` *must* be set
* If adding codes to an existing codelist the codelist filename *must* append `+` to the start of the core codelist filename, for example `+documentType.csv`
### Field and code names
* Definition names *must* be UpperCamelCase
* Field names *must* be lowerCamelCase
* Definition and field names *must* contain only ASCII alphabetical letters
* If an acronym is used within a field or definition name, the acronym *should* be all UPPERCASE, unless it is at the beginning of the name, in which case it *should* be all lowercase.
### Changes to existing fields
A conformant extension *should not*:
* Delete properties or definitions from the OCDS schema.
* Change the value of keywords in the OCDS schema. If an extension desires to document further usage of a field in the OCDS, it should do so through documentation, rather than changing the field's `description` keyword.
### New fields
A conformant extension *must*:
* Set the `title`, `description` and `type` keywords for all new definitions and properties
* Set `$ref` for fields that reuse a definition in the OCDS schema.
* Set the `items` keyword for properties of `type` "array"
* Not include types other than "array", "number" and/or "string" in the `types` keyword under an `items` keyword.
* Ensure that any definition referenced in a property of `type` array, whose `wholeListMerge` and `omitWhenMerged` keywords are unset or set to `false`, has a required `id` property.
### Codelists
A conformant extension *must*:
* Set `enum` for properties with `openCodelist` set to `false`
* Not set `enum` for properties with `openCodelist` set to `true`
* Set `codelist` and `openCodelist` for properties that set `enum`.
* Document titles and descriptions for `enum` values as a CSV codelist.
* Append "+" to the start of the codelist filename when adding codes to an existing codelist, e.g. `+documentType.csv`.
### Naming conventions
A conformant extension *must*:
* Use lowerCamelCase for property names, e.g. `tenderPeriod`
* Use UpperCamelCase for definition names, e.g. `RelatedProcess`
* Use only ASCII alphabetical letters for property and definition names
A conformant extension *should* use UPPERCASE for acronyms within property or definition names, unless the acronym is at the beginning of the name, in which case it *should* be all lowercase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unresolved this just so that it's visible for James when he comes to review

Copy link
Contributor

@duncandewhurst duncandewhurst left a comment

Choose a reason for hiding this comment

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

Happy for you to request a review from James, if you accept my suggestion. It's probably worth flagging to him the reasons for using JSON Schema terminology expressed in #1715 (comment).

docs/schema/conformance_and_extensions.md Outdated Show resolved Hide resolved
@odscjen odscjen requested a review from jpmckinney November 21, 2024 09:53
@jpmckinney
Copy link
Member

an additional PR to ensure this is consistent with the style guide may be needed

Can you describe in more detail?

@odscjen
Copy link
Contributor Author

odscjen commented Nov 22, 2024

an additional PR to ensure this is consistent with the style guide may be needed

Can you describe in more detail?

This is in reference to #582 (comment) - some of the detail added in this PR is also covered in the style guide, e.g. 2 of the rules in the Naming Conventions section are repeated in https://ocds-standard-development-handbook.readthedocs.io/en/latest/meta/schema_style_guide.html#field-and-code-names, while the third rule doesn't appear to be in that part of the style guide at all.

I've not done a full review to check which bullet points are already in the style guide and which are new and therefore what might need to be done to ensure consistency across the 2 resources. I can do so as part of this PR but I feel it might be easier as a separate issue and PR so we can discuss the best way to do it

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.

3 participants