-
Notifications
You must be signed in to change notification settings - Fork 15
2021 11 schema update #305
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
Conversation
schema/360-giving-schema.json
Outdated
| "ZW" | ||
| ], | ||
| "description": "The ISO Country Code of the location of this activity.", | ||
| "description": "The 2-character ISO Country Code of the location of this activity. Taken from [ISO_3166-1_alpha-2](https://www.iso.org/iso-3166-country-codes.html)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why underscores are being used here - the name of the code is ISO 3166-1 alpha-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from the housekeeping doc. I'll amend.
| "null" | ||
| ], | ||
| "description": "The latitude of a point location", | ||
| "description": "The latitude of a point location. Using [WGS 84](https://en.wikipedia.org/wiki/World_Geodetic_System) standard, for example 51.501502", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If one was being very pedantic, one might argue that this is a backwards-incompatible change because previously-allowed values are no longer allowed.
However, I find it incredibly unlikely that anyone is using anything other than WGS 84, so I doubt this is a real issue. I like the provision of an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @robredpath I raised exactly this question in particular in a planio ticket - 30810 - as I believe this isn't a backwards incompatible change to the schema, but a clarification of guidance. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KDuerden I agree - for all practical purposes, it's backward-compatible and so it's fine. I will buy, and eat, a hat if this change actually breaks anyone's use case.
The WGS 84 vs EPSG 4326 question is an even smaller nit to pick, but we might as well get it right. @Lathrisk will be able to tell us what to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any comments about EPSG that have an impact on this PR or are they issues for separate consideration?
schema/360-giving-schema.json
Outdated
| "null" | ||
| ], | ||
| "description": "A code referring to a geographical area, drawn from an established gazetteer. For example, the code for a local authority ward, or parliamentary constituency.", | ||
| "description": "A code referring to a geographical area, drawn from an established gazetteer such as the ONS Register of Geographic Codes in the UK. For example, the code for a local authority ward, or parliamentary constituency.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link?
schema/360-giving-schema.json
Outdated
| "streetAddress": { | ||
| "type": "string", | ||
| "description": "Building number and street name.", | ||
| "description": "Recipient organisation address Building number and street name.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The capital B on Building is grammatically incorrect with this new information prepended
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(same for lines below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cheers Rob, this was following the Housekeeping doc and I did notice this. I'll amend appropriately.
| "issued": { | ||
| "title": "Issued", | ||
| "description": "The date that this data package was issued.", | ||
| "description": "The date that this data package was issued. The date must be written as YYYY-MM-DD, or in full date-time format", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we link to JSON Schema's date-time format ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is reasonable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't link to JSON schema's date formatting in any of the other parts of the schema that use this formatting so seems odd to do so in this case only. We have field guidance on date and date-time, so I think that provides sufficient supporting information.
schema/360-giving-schema.json
Outdated
| "ZW" | ||
| ], | ||
| "description": "The ISO Country Code of the location of this activity.", | ||
| "description": "The 2-character ISO Country Code of the location of this activity. Taken from [ISO-3166-1-alpha-2](https://www.iso.org/iso-3166-country-codes.html)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was agreed to use link https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2 rather than ISO link because ISO is not open.
schema/360-giving-schema.json
Outdated
| "null" | ||
| ], | ||
| "description": "A code referring to a geographical area, drawn from an established gazetteer. For example, the code for a local authority ward, or parliamentary constituency.", | ||
| "description": "A code referring to a geographical area, drawn from an established gazetteer such as the [ONS Register of Geographic Codes](https://data.gov.uk/dataset/389511fa-eb72-4657-a2ef-b3c982ec4c0e/register-of-geographic-codes-april-2020-for-the-united-kingdom) in the UK. For example, the code for a local authority ward, or parliamentary constituency.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know if that link from ONS will persist? As it is time specific I would be more comfortable using a wiki link to generic information - eg https://en.wikipedia.org/wiki/ONS_coding_system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd second using a Wiki link, I can modify this PR if needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The canonical link for the Register of Geographic Codes would be this one: https://geoportal.statistics.gov.uk/datasets/register-of-geographic-codes-june-2021-for-the-united-kingdom/about - but that changes every time they publish a new one, and the links are likely to change (plus to geoportal is a slightly rubbish js-heavy site so it's never 100% links will work anyway). So wiki probably makes sense.
schema/360-giving-schema.json
Outdated
| "streetAddress": { | ||
| "type": "string", | ||
| "description": "Building number and street name.", | ||
| "description": "Recipient organisation address building number and street name.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these fields are shared by Recipient and Funder organisation? If so, these need to be edited to removed 'Recipient from the start of all descriptions - so it will be generic.
schema/360-giving-schema.json
Outdated
| "addressLocality": { | ||
| "type": "string", | ||
| "description": "City or town.", | ||
| "description": "Recipient organisation address city or town.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove Recipient
schema/360-giving-schema.json
Outdated
| "addressRegion": { | ||
| "type": "string", | ||
| "description": "County", | ||
| "description": "Recipient organisation address county", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove Recipient
schema/360-giving-schema.json
Outdated
| "addressCountry": { | ||
| "type": "string", | ||
| "description": "Country", | ||
| "description": "Recipient organisation address country", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove Recipient
schema/360-giving-schema.json
Outdated
| "postalCode": { | ||
| "type": "string", | ||
| "description": "Postal code (please try and provide a post code whenever possible)", | ||
| "description": "Recipient organisation address postal code.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove Recipient
KDuerden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've checked these changes and I'm happy with them. I'm approving this in my capacity as member of 360Giving team, as the first stage in PATCH approval process, which requires sign off from 360Giving team and a member of the Stewardship committee.
PR made based on Planio-29617 as part of the housekeeping review and ahead of the SC meeting. This is for the changes to the schema only, documentation will be made in a separate pull request.