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

Add conditional requirement #63

Merged
merged 15 commits into from
Feb 21, 2022

Conversation

tdelmas
Copy link
Collaborator

@tdelmas tdelmas commented Feb 8, 2022

Supersede #59 at the request of @PierrickP

This PR adds a first batch of conditional checks such as:

  • Files required depending on the content of other files
  • Fields required depending on the content of other files
  • Value of fields depending on the content of other files (e.g. the validity of ids referring to an object in another file)

From @isabelle-dr #59 (comment) :

  • default_pricing_plan_id in vehicle_types.json is checked (requirement and value)
  • vehicle_types_available in station_status.json requirement is checked
  • num_docks_available in station_status.json can't be automatically checked because it depends on something not defined in the files: REQUIRED except for stations that have unlimited docking capacity. It could be done in a separate PR as a checkbox option in the validator UI such as "[ ] All stations limited docking capacity".
  • vehicle_docks_available in station_status.json can't be automatically checked because it depends on something not defined in the files: REQUIRED in feeds where [...] certain docks are only able to accept certain vehicle types. It could be done in a separate PR as a checkbox option in the validator UI such as "[ ] All stations accepts all vehicle types".
  • system_id in free_bike_status.json is not touched by that PR.
  • lat and lon in free_bike_status.json : As the conditional is between two fields of the same object, it has been fixed in the schemas: Improved schema gbfs-json-schema#66
  • vehicle_type_id in free_bike_status.json is checked (requirement and value)
  • current_range_meters in free_bike_status.json requirement is checked
  • the file vehicle_types.json conditional requirement is checked

Also, this PR adds the following checks:

  • The conditional requirement of system_pricing_plans.json
  • The conditional requirement and value of default_pricing_plan_id in vehicle_types.json
  • The conditional requirement of rental_apps in system_information.json
  • The conditional requirement of rental_apps in system_information.json
  • The conditional requirement of rental_apps.ios.store_uri and rental_apps.android.store_uri in system_information.json

Other changes on this PR:

PierrickP and others added 12 commits February 1, 2022 15:46
On `free_bike_status.json`
+ `vehicle_type_id` if has a `vehicle_types.json`
+ `current_range_meters` if the vehicle_type_id has not the `human` propulsion

Set `vehicle_types.json` required if `free_bike_status.json` has `vehicle_type_id`

issue MobilityData#54
…6ec1

90a6ec1 Update README.md
c05169e Create README.md (MobilityData#64)
b871a0e Add GitHub Actions to validate schemas (MobilityData#54)
61c4de1 Fix invalid json, no trailing commas (MobilityData#63)

git-subtree-dir: gbfs-validator/versions/schemas
git-subtree-split: 90a6ec18bb7d23b5eba8e400b5df549d254e9fd2
@netlify
Copy link

netlify bot commented Feb 8, 2022

✔️ Deploy Preview for unruffled-hugle-914373 ready!

🔨 Explore the source changes: a77e9ad

🔍 Inspect the deploy log: https://app.netlify.com/sites/unruffled-hugle-914373/deploys/6203e5cbcd7c790008157b3a

😎 Browse the preview: https://deploy-preview-63--unruffled-hugle-914373.netlify.app

@netlify
Copy link

netlify bot commented Feb 8, 2022

✔️ Deploy Preview for gbfs-validator ready!

🔨 Explore the source changes: a77e9ad

🔍 Inspect the deploy log: https://app.netlify.com/sites/gbfs-validator/deploys/6203e5cb8dde2e000718b313

😎 Browse the preview: https://deploy-preview-63--gbfs-validator.netlify.app/

@netlify
Copy link

netlify bot commented Feb 8, 2022

❌ Deploy Preview for competent-payne-690ca9 failed.

🔨 Explore the source changes: a77e9ad

🔍 Inspect the deploy log: https://app.netlify.com/sites/competent-payne-690ca9/deploys/6203e5cbe0251800083d39cf

@netlify
Copy link

netlify bot commented Feb 8, 2022

❌ Deploy Preview for kind-pike-a3f3f8 failed.

🔨 Explore the source changes: a77e9ad

🔍 Inspect the deploy log: https://app.netlify.com/sites/kind-pike-a3f3f8/deploys/6203e5cbf7104700076754db

@netlify
Copy link

netlify bot commented Feb 8, 2022

❌ Deploy Preview for wizardly-lichterman-770f54 failed.

🔨 Explore the source changes: a77e9ad

🔍 Inspect the deploy log: https://app.netlify.com/sites/wizardly-lichterman-770f54/deploys/6203e5cb53e2ee0007334580

@netlify
Copy link

netlify bot commented Feb 8, 2022

❌ Deploy Preview for wizardly-engelbart-5c48ca failed.

🔨 Explore the source changes: a77e9ad

🔍 Inspect the deploy log: https://app.netlify.com/sites/wizardly-engelbart-5c48ca/deploys/6203e5cbbb27860007ebf127

@PierrickP PierrickP self-assigned this Feb 8, 2022
@isabelle-dr
Copy link
Contributor

Thank you for working on this! 🙏

@PierrickP PierrickP marked this pull request as ready for review February 9, 2022 16:03
@PierrickP PierrickP self-requested a review February 9, 2022 16:13
@isabelle-dr
Copy link
Contributor

Hello,

This contribution is amazing 🔥
I opened #64 to update the documentation so it reflects all this work. I know we require one review before merging - can you wait for a review by a MobilityData as well in the future?
🙏🙏🙏

@nbdh
Copy link
Contributor

nbdh commented Sep 22, 2022

Hi @PierrickP,

thanks for your work!

I suspect that this change broke #41. For v2.2 feeds the vehicle_types.json is thought to be required again, even if the criteria stated in gbfs.md

REQUIRED of systems that include information about vehicle types in the free_bike_status file.

is not given.

Example feed: https://gbfs-validator.netlify.app/?url=https%3A%2F%2Fgbfs.nextbike.net%2Fmaps%2Fgbfs%2Fv2%2Fnextbike_le%2Fgbfs.json
which states

Missing file and required

for the vehicle_types.json.

Should I open a new issue?

Best regards

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.

None yet

4 participants