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

[Feature] JSON Schema #25

Merged
merged 7 commits into from
Nov 6, 2023
Merged

Conversation

jenkin
Copy link
Contributor

@jenkin jenkin commented Oct 10, 2023

This PR adds a complete JSON Schema that is compliant with the spec. It restricts the official GeoJSON schema defining two additional constraints related to geocode objects in GeoJSON FeatureCollection and Feature objects.

It defines a JSON Schema for semver (see semver.org/issues/431) and for geohash.

It adds a simple toolchain managed with make to dereference and bundle external schemas (geojson as remote dependency, semver and geohash as local ones) and to compute checksum of distributed file. Human-editable files are in src/ folder, final bundle is in draft/ folder with the human-readable spec.

It also adds a simple README in root folder.

SUGGESTION: enable Github Pages on master branch to publish the schema at https://geocoders.github.io/geocodejson-spec/draft/geocodejson.schema.json with proper mime type.

Refs: #24

@yohanboniface
Copy link
Member

Thanks for this work @jenkin ! Open API schema and README looks good to me! :)

I'm not exactly getting why git hooks are involved here ? Why not for example a "simple" make bundle command ? Thanks for your lights :)

@jenkin
Copy link
Contributor Author

jenkin commented Oct 11, 2023

Git hooks ensure that no commit can contain invalid or outdated schema. But we can disable them and use make rules manually to reduce complexity, if you are ok I can add a commit before merge.

@yohanboniface
Copy link
Member

The simpler, the better :) So I'd better go with a documented release process and make utilities, yep, if that's ok for you :)

@jenkin
Copy link
Contributor Author

jenkin commented Oct 11, 2023

Et voilà! :)

@yohanboniface
Copy link
Member

Grazie! :)

Looks good to me. @lonvia @nlehuby up for a look ? :)

Copy link

@nlehuby nlehuby left a comment

Choose a reason for hiding this comment

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

nice job, thanks 👍

@jenkin
Copy link
Contributor Author

jenkin commented Nov 3, 2023

Hi @yohanboniface @lonvia , any news? In the meantime I've published the draft in one of my repo... ;) Best!

jenkin added a commit to sparkfabrik/nominatim-openapi that referenced this pull request Nov 6, 2023
jenkin added a commit to sparkfabrik/nominatim-openapi that referenced this pull request Nov 6, 2023
@lonvia
Copy link

lonvia commented Nov 6, 2023

Looks okay. Just one question for my understanding: the human-readable draft/README.md and the schema are completely separate? Do we need to make manually sure that they are the same?

@jenkin
Copy link
Contributor Author

jenkin commented Nov 6, 2023

Just one question for my understanding: the human-readable draft/README.md and the schema are completely separate? Do we need to make manually sure that they are the same?

Yes. Actually the spec is quite simple and mainly written as comments in code blocks, so we can use the json schema as source of truth and generate the human readable spec from it with a custom script. Maybe an additional task for the next v1 release (see #26)?

@yohanboniface yohanboniface merged commit 1e30b4b into geocoders:master Nov 6, 2023
@yohanboniface
Copy link
Member

Good point! But I don't know how to keep those in sync without setting up a complex workflow, so I'd say it's acceptable at this stage.

@jenkin added you as owner :)

jenkin added a commit to sparkfabrik/nominatim-openapi that referenced this pull request Nov 7, 2023
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.

4 participants