feat: Added route names component#1469
feat: Added route names component#1469cka-y merged 3 commits intoMobilityData:masterfrom cka-y:issue/1464/add-route-names
Conversation
|
Thanks for opening this pull request! You're awesome. We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of titles with semantic prefixes:
|
|
|
fix: short and long name needs to be defined for the same entry
| if (routeTable.hasColumn(GtfsRoute.ROUTE_SHORT_NAME_FIELD_NAME) | ||
| && routeTable.hasColumn(GtfsRoute.ROUTE_LONG_NAME_FIELD_NAME)) | ||
| return routeTable.getEntities().stream() | ||
| .anyMatch(route -> route.hasRouteShortName() && route.hasRouteLongName()); |
There was a problem hiding this comment.
It's unclear to me if this should be an AND or an OR. The issue is a bit ambiguous on the subject IMO:
We consider this component present if both the fields route_short_name and route_long_name in routes.txt are present, and if they have at least one value defined.
Does that mean that there should be one value for each field, or one value for either one of the fields?
Logically it seems that if you have either a short name or a long name it should be accepted. But it should be clarified.
There was a problem hiding this comment.
I actually checked with Isabelle regarding that :
|
I would suggest briefly explaining in the issue the manual testing you did if any. Did you modify a dataset to simulate the situation? How was it modified? Maybe include the dataset you used? Or simple paste a couple of lines from the modified file form the dataset.. |
|
🥳 Congrats on getting your first pull request merged! |
3 participants
Summary:
Add a "Route Names" GTFS Component #1464
Closes #1464
Expected behavior:
"Route Names" component should be added to this list. We consider this component present if both the fields route_short_name and route_long_name in routes.txt are present, and if they have at least one value defined.
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anything