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

vehicle_types.json is not required #41

Merged
merged 2 commits into from
Nov 2, 2021

Conversation

nbdh
Copy link
Contributor

@nbdh nbdh commented Oct 20, 2021

Hi @isabelle-dr and contributors,

thanks for the good work!

I failed to raise this during yesterdays workshop as I only noticed after the slot was finished, but in my opinion, the validator should not declare potentially valid feeds as invalid. A feed rather should be considered invalid only if it clearly is not, while stating it would be valid still does not mean it could not potentially be invalid (e.g. because free_bike_status refers to vehicle types that are not declared).

Full disclosure: Our implementation of vehicle_types.json is still in progress and even though the spec says we should be publishing it, we are not required to do so and yet our feeds are considered invalid ;)

@CLAassistant
Copy link

CLAassistant commented Oct 20, 2021

CLA assistant check
All committers have signed the CLA.

@netlify
Copy link

netlify bot commented Oct 20, 2021

✔️ Deploy Preview for wizardly-lichterman-770f54 canceled.

🔨 Explore the source changes: 414bd5a

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

@netlify
Copy link

netlify bot commented Oct 20, 2021

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

🔨 Explore the source changes: 414bd5a

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

@netlify
Copy link

netlify bot commented Oct 20, 2021

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

🔨 Explore the source changes: 414bd5a

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

@netlify
Copy link

netlify bot commented Oct 20, 2021

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

🔨 Explore the source changes: 414bd5a

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

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

@netlify
Copy link

netlify bot commented Oct 20, 2021

❌ Deploy Preview for kind-pike-a3f3f8 failed.

🔨 Explore the source changes: 414bd5a

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

@PierrickP
Copy link
Collaborator

@nbdh Hello,
I would like to try implementing the right conditional rule (between files).
If i'm sure it's 100% impossible, we can be more permissive on this and merge this PR

@nbdh
Copy link
Contributor Author

nbdh commented Nov 2, 2021

Hi @PierrickP,

thanks for your reply. Of course properly resolving the "to do" by implementing the conditional rule is the way to go.

Still I'd argue that in the meantime, no potentially valid feeds shall be declared invalid. Imho, even if the validator states that a feed is valid, it might later turn out it is not, while stating that it is not valid should mean that it definitely does not match the specification.

@isabelle-dr
Copy link
Contributor

Hi @nbdh and @PierrickP.

I'm OK merging this PR and having that file as optional until we find a way to write this rule properly in the validator.
I opened issue #54 to keep track of it, merging this PR!

@isabelle-dr isabelle-dr merged commit dfd3d71 into MobilityData:master Nov 2, 2021
@nbdh nbdh mentioned this pull request Sep 22, 2022
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