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.

Sign up for GitHub

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

Jump to bottom

Update Json Schema #67

Merged
merged 12 commits into from
Mar 30, 2022
Merged

Update Json Schema #67

merged 12 commits into from
Mar 30, 2022

Conversation

isabelle-dr
Copy link
Contributor

Closes #65
This change includes:

@isabelle-dr
Squashed 'gbfs-validator/versions/schemas/' changes from 90a6ec1..99a…
33f8032 
…54d4

99a54d4 Update V2.3-RC to V2.3-RC2 (#67)
3d8926c Revert "Update name tag to 2.3-RC2"
ed57408 Revert "Changes to return_types"
2749dce Revert "Extend vehicle types"
7029048 Revert "Update vehicle_types.json with car sharing extention"
e249fc2 Revert "fix typo in vehicle_types.json"
8b5f600 fix typo in vehicle_types.json
539f06f Update vehicle_types.json with car sharing extention
5ec98f3 Extend vehicle types
c8a4389 Changes to return_types
6eacd24 Update name tag to 2.3-RC2
f5e1a59 Improved schema (#66)

git-subtree-dir: gbfs-validator/versions/schemas
git-subtree-split: 99a54d4b9dd0e7dc9165f19182047dabc438593c
@isabelle-dr
Update JSON Schema with latest fixes and v2.3-RC2
47379f9
@netlify
Copy link

netlify bot commented Mar 16, 2022 •
edited
Loading

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

Name Link
🔨 Latest commit 1396f27
🔍 Latest deploy log https://app.netlify.com/sites/wizardly-engelbart-5c48ca/deploys/623b2a6dbf17810008a74957

@netlify
Copy link

netlify bot commented Mar 16, 2022 •
edited
Loading

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

Name Link
🔨 Latest commit 1396f27
🔍 Latest deploy log https://app.netlify.com/sites/competent-payne-690ca9/deploys/623b2a6deffecd0008785947

@netlify
Copy link

netlify bot commented Mar 16, 2022 •
edited
Loading

❌ Deploy Preview for kind-pike-a3f3f8 failed.

Name Link
🔨 Latest commit 1396f27
🔍 Latest deploy log https://app.netlify.com/sites/kind-pike-a3f3f8/deploys/623b2a6d5d09230009edccb6

@netlify
Copy link

netlify bot commented Mar 16, 2022 •
edited
Loading

✅ Deploy Preview for unruffled-hugle-914373 ready!

Name Link
🔨 Latest commit 1396f27
🔍 Latest deploy log https://app.netlify.com/sites/unruffled-hugle-914373/deploys/623b2a6d2f2e830009bee3ba
😎 Deploy Preview https://deploy-preview-67--unruffled-hugle-914373.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 16, 2022 •
edited
Loading

✅ Deploy Preview for gbfs-validator ready!

Name Link
🔨 Latest commit 1396f27
🔍 Latest deploy log https://app.netlify.com/sites/gbfs-validator/deploys/623b2a6dada6cc0009cdd42c
😎 Deploy Preview https://deploy-preview-67--gbfs-validator.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 16, 2022 •
edited
Loading

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

Name Link
🔨 Latest commit 1396f27
🔍 Latest deploy log https://app.netlify.com/sites/wizardly-lichterman-770f54/deploys/623b2a6d229fa90009ca3d9a

@isabelle-dr isabelle-dr requested a review from PierrickP March 16, 2022 13:28
@isabelle-dr
Update README & fix the link to the Json Schema
58cbef6
@isabelle-dr
Squashed 'gbfs-validator/versions/schemas/' changes from 99a54d4..5e0…
a279014 
…2044

5e02044 Fix type (#70)

git-subtree-dir: gbfs-validator/versions/schemas
git-subtree-split: 5e02044c90491f2e2804b619b8030692b39cf4a0
@isabelle-dr
Update after type fix in #gbfs-json-schema/pull/70
28db2a7
@isabelle-dr isabelle-dr requested a review from testower March 22, 2022 15:23
@testower
Copy link
Collaborator

@isabelle-dr I believe you have a unit test failing because it's trying to read the schema from the wrong path due to this fixture:

https://github.com/MobilityData/gbfs-validator/blob/master/gbfs-validator/__test__/fixtures/plan_id.js#L10

Change the version on this line to '2.3-RC2' and I think it will pass again.

testower
testower approved these changes Mar 22, 2022
View reviewed changes
Copy link
Collaborator

@testower testower left a comment

Choose a reason for hiding this comment

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

Looks good, apart from the failing unit test, which should be easy to fix 👍

@isabelle-dr
Copy link
Contributor Author

@PierrickP Still failing 😕 I can't seem to find why.
Also: I renamed the folder in the schemas V2.3-RC2, do you think this would cause an issue with the version drop-down on the website?

@isabelle-dr
Copy link
Contributor Author

Well it seems like it does 🙈. I updated the Validator.vue file to match the new name. I still can't get why the unit tests are failing.

@testower testower self-requested a review March 23, 2022 09:06
testower
testower suggested changes Mar 23, 2022
View reviewed changes
Copy link
Collaborator

@testower testower left a comment

Choose a reason for hiding this comment

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

Actually, I think we need to hold this up. There is a breaking change in vehicle_types.json here, which is the requirement of default_pricing_plan_id. Mitch was going to look into it. Meanwhile, I'm having a look at the tests.

@testower
Copy link
Collaborator

Here is a PR into this branch that fixes the issue: #68

I still think we need to remove the requirement condition on default_pricing_plan_id though.

@PierrickP
Copy link
Collaborator

Thanks @testower, i merged your PR to fix this PR.

@isabelle-dr you renamed RC to RC2. Should we NOT keep old RCs ?

@testower
Copy link
Collaborator

@isabelle-dr default_pricing_plan_id has been made completely optional in 2.3-RC2 now, can you update this PR so we can get it merged?

@isabelle-dr
Copy link
Contributor Author

Hello!

@PierrickP after a discussion with @mplsmitch to confirm, we decided to replace V2.3 with V2.3-RC2 because they have been released at dates that are really close. We don't really want to encourage users to implement V2.3-RC.

@testower the requirement condition on default_pricing_plan_id is covered by a custom rule that has been added in PR #63. We can merge this PR and remove this rule in another PR. @PierrickP, would you be able to do it?

@isabelle-dr isabelle-dr merged commit 839e558 into master Mar 30, 2022
@PierrickP
Copy link
Collaborator

@isabelle-dr Thanks
Ok for the v2-3-RC and i ll make a PR to update the version requirement according to the latest change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@testower testower testower approved these changes

@PierrickP PierrickP Awaiting requested review from PierrickP

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update schemas
3 participants
@isabelle-dr @testower @PierrickP