feat: recommended file annotation #877#1123
feat: recommended file annotation #877#1123KClough wants to merge 1 commit intoMobilityData:masterfrom
Conversation
5e59b46 to
b84e7e2
Compare
There was a problem hiding this comment.
Thanks you for this great contribution @KClough!
I had a quick look because I know you are planning on adding more rules (🎉!) so I wanted to see if there are corrections that would apply to your next contributions.
Besides the comments that I left in-line, I have two main pieces of feedback here:
- There are two things included in this PR: the addition of the @recommended annotation in this validator, and the addition of a new validator notice. If this isn't too much effort on your end, I would recommend splitting this PR in two to make sure we aren't missing anything during the reviews.
- As mentioned in the issue this PR is closing (#887), I believe there is no mention in (1) the spec or (2) the Best Practices that
shapes.txtshould be included. This validator is currently based on them, and the few exceptions we have are rules that are related to good data design practices (e.g. checking that pathway is not a loop). The recommendation onshapes.txtshould be added in the Best Practices first before being added here, so I recommend removing it from this PR.
Our tech lead will have a look shortly to look at the technical aspects.
Thank you so much again to contribute to this tool, let me know if you have any questions!
| * <p>Example. | ||
| * | ||
| * <pre> | ||
| * {@literal @}GtfsTable("agency.txt") |
There was a problem hiding this comment.
I would have a real example here with a file where we use the @recommended annotation. It could be feed_info.txt.
RULES.md
Outdated
| | [`LocationWithUnexpectedStopTimeNotice`](#LocationWithUnexpectedStopTimeNotice) | A location in `stops.txt` that is not a stop is referenced by some `stop_times.stop_id`. | | ||
| | [`MissingCalendarAndCalendarDateFilesNotice`](#MissingCalendarAndCalendarDateFilesNotice) | Missing GTFS files `calendar.txt` and `calendar_dates.txt`. | | ||
| | [`MissingLevelIdNotice`](#MissingLevelIdNotice) | `stops.level_id` is conditionally required. | | ||
| | [`MissingRecommendedFileNotice`](#MissingRecommendedFileNotice) | A recommended file is missing. | |
There was a problem hiding this comment.
We are missing the part this link brings the user to. It is usually a longer description and it appears lower in the document. It could be:
MissingRequiredFileNotice
The GTFS Best Practice recommends a file be included, but it's missing.
References:
docs/NOTICES.md
Outdated
| | `location_without_parent_station` | [`LocationWithoutParentStationNotice`](#LocationWithoutParentStationNotice) | | ||
| | `missing_calendar_and_calendar_date_files` | [`MissingCalendarAndCalendarDateFilesNotice`](#MissingCalendarAndCalendarDateFilesNotice) | | ||
| | `missing_level_id` | [`MissingLevelIdNotice`](#MissingLevelIdNotice) | | ||
| | `missing_recommended_file` | [`MissingRecommendedFileNotice`](#MissingRecommendedFileNotice) |
There was a problem hiding this comment.
Similar to my comment in RULES.md, we are missing the part at the bottom of the document, where we give the user a table showing what is included in the JSON schema for this notice.
15cb119 to
b8116f6
Compare
b8116f6 to
5d191ee
Compare
Summary:
Resolves #877
Adds a new annotation for recommended files and applies this annotation to
feed_info.txtandshapes.txtExpected behavior:
A new validator warning is generated when either
feed_info.txtorshapes.txtis missingPlease make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anythingIn liue of a screenshot, here is the sample output from the validator for a data set with missing
feed_info.txtandshapes.txt:{ "notices": [ { "code": "missing_recommended_file", "severity": "WARNING", "totalNotices": 2, "sampleNotices": [ { "filename": "shapes.txt" }, { "filename": "feed_info.txt" } ] } ]}