feat: Add a rule that stations (location_type 1) must be the parent_station of some stop (location_type 0).#1493
Conversation
This renames ParentLocationTypeValidator to the more general name ParentStationValidator. For unit testing it is now necessary to consider two types of notices, so all "contains()" calls are changed to "containsExactly()" or "containsExactlyElementsIn" to be sure that additional notices are not ignored.
davidgamez
left a comment
davidgamez
left a comment
There was a problem hiding this comment.
Hi @bradyhunsaker, few changes have been merged since your first PR regarding the notice documentation generation that impacts your contribution. I added a few comments to align this PR with the latest changes before approval.
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java
Outdated
Show resolved
Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java
Show resolved
Hide resolved
…rentStationValidator.java Co-authored-by: David Gamez <[email protected]>
|
Thanks, David! I accepted your suggested edit and removed the super() call. Any other comments? |
Thanks, @bradyhunsaker. One more ask, Can you review the formatting(./gradlew goJF) and class imports? I'm sorry I typed my suggestions on Github and didn't have the proper formatting and missed an import. |
|
OK. I'm wrestling with google-java-format right. I'm getting an error like this one: I'll try again tomorrow. |
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java
Outdated
Show resolved
Hide resolved
|
Thanks. I managed to run google-java-format from a separate download rather than using gradlew. Of course it just made the change you gave. |
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java
Show resolved
Hide resolved
…rentStationValidator.java Co-authored-by: David Gamez <[email protected]>
|
Thanks for the documentation fix. The end-to-end test is now failing in validate_gradle_wrapper, but the error message looks like it's something with the system itself rather than the PR. I'm not familiar with this testing setup, so I could be wrong about that. |
This is an intermittent issue downloading the gradle binary, I retried it and passed without issues. I'm approving and merging right now. Thanks for taking it to the end of the line!! |
|
🥳 Congrats on getting your first pull request merged! |
2 participants
Summary:
Part of #153.
The new notice is of type INFO because there is nothing in the specification or best practices stating that stations should have stops. There is a discussion about changing this in #1348.
As part of the change, this renames ParentLocationTypeValidator to the more general name ParentStationValidator.
For unit testing it is now necessary to consider two types of notices, so all "contains()" calls are changed to "containsExactly()" or "containsExactlyElementsIn()" to be sure that additional notices are not ignored.
Expected behavior:
If a location of type 1 (station) never appears as the parent station of a location of type 0 (stop/platform), then issue a notice.
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anything