-
Notifications
You must be signed in to change notification settings - Fork 12
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
added support for v3.1-RC #185
Conversation
✅ Deploy Preview for gbfs-validator ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
"dependencies": { | ||
"reservation_price_flat_rate": { "not": { "required": ["reservation_price_per_min"] } }, | ||
"reservation_price_per_min": { "not": { "required": ["reservation_price_flat_rate"] } } | ||
} |
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.
@richfab I added this to the schema which says: either reservation_price_flat_rate
or reservation_price_per_min
is present but not both of them together. Let me know if an addition like this to the schema is possible or should this be done in the form of a validation patch
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.
Nice improvement! Great use of the JSON schema features. I'll add it to https://github.com/MobilityData/gbfs-json-schema. Thanks 🙏
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.
PR is up for review: MobilityData/gbfs-json-schema#125
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 the schemas duplicated here? and in the schema repo? How do we make sure they don't diverge?
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.
Yes, currently the schemas in https://github.com/MobilityData/gbfs-validator/ are duplicated from https://github.com/MobilityData/gbfs-json-schema/.
Open to suggestions to make sure they are always in sync.
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.
Two possible ways among others:
- download them from the other repo (how it's done in entur validator)
- use git submodule
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.
Created an issue for this: #186
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.
LGTM 💯
Thanks @Alessandro100 🙏
Fixes #184
Added support for GBFS v3.1-RC
New custom rules added
reservation_price_per_min
orreservation_price_flat_rate
allowed in plansreservation_price_per_min
orreservation_price_flat_rate
is present in the associated vehicle type, thendefault_reserve_time
becomes requiredKey changed files
gbfs-validator/versions/partials/v3.1-RC/vehicle_types/default_reserve_time_require.js
gbfs-validator/gbfs.js
Key test files
gbfs-validator/test/fixtures/conditional_default_reserve_time.js
gbfs-validator/test/gbfs.test.js
References
https://gbfs.org/specification/
https://github.com/MobilityData/gbfs/blob/v3.1-RC/gbfs.md