fix: Add notice for missing stop name for specific location types#1316
Conversation
|
|
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/StopNameValidator.java
Outdated
Show resolved
Hide resolved
bdferris-v2
left a comment
bdferris-v2
left a comment
There was a problem hiding this comment.
Modulo the one comment, this LGTM.
isabelle-dr
left a comment
isabelle-dr
left a comment
There was a problem hiding this comment.
The new rule LGTM, we just need the RULES.md update :)
More a general thought than something specific to this PR: I wonder if we want to formalize our preferred approach between:
- generating a custom notice for the conditionally required fields
- re-using
MissingRequiredField
@isabelle-dr updated! |
|
@isabelle-dr apologies I missed the RULES.md update on this. I've taken a stab at adding a unit-test to catch this sort of thing in #1331. |
isabelle-dr
left a comment
isabelle-dr
left a comment
There was a problem hiding this comment.
Just two propositions for the definitions, thanks for updating this so quickly @briandonahue!
Why are there also modifications to the java files in this PR?
Co-authored-by: isabelle-dr <[email protected]>
@isabelle-dr this PR contains the updated validation and the RULES.md update to go along with it. Should they be separate? |
isabelle-dr
left a comment
isabelle-dr
left a comment
There was a problem hiding this comment.
Sorry, I got mixed up with #1330 🙈.
This is perfect! LGTM
|
Can you update this branch with the latest master? |
…bilityData#1316) * Fix: Add notice for missing stop name for specific location types * switch to isEmpty() * Address PR feedback, cleanup test for readability * Update RULES.md * Apply suggestions from code review Co-authored-by: isabelle-dr <[email protected]> --------- Co-authored-by: Kevin Clough <[email protected]> Co-authored-by: isabelle-dr <[email protected]>
…bilityData#1316) * Fix: Add notice for missing stop name for specific location types * switch to isEmpty() * Address PR feedback, cleanup test for readability * Update RULES.md * Apply suggestions from code review Co-authored-by: isabelle-dr <[email protected]> --------- Co-authored-by: Kevin Clough <[email protected]> Co-authored-by: isabelle-dr <[email protected]>
…bilityData#1316) * Fix: Add notice for missing stop name for specific location types * switch to isEmpty() * Address PR feedback, cleanup test for readability * Update RULES.md * Apply suggestions from code review Co-authored-by: isabelle-dr <[email protected]> --------- Co-authored-by: Kevin Clough <[email protected]> Co-authored-by: isabelle-dr <[email protected]>
6 participants
Summary:
Adds validation for required stop name for STOP, STATION, ENTRANCE location types. Closes #1078
Expected behavior:
Should generate an Error notice if stop name is missing for the above location types.
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anything