Add trip short name #588
Add trip short name #588
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #588 +/- ##
===========================================
- Coverage 43.89% 15.80% -28.09%
===========================================
Files 315 315
Lines 17270 16100 -1170
Branches 5269 4896 -373
===========================================
- Hits 7581 2545 -5036
- Misses 8443 11570 +3127
- Partials 1246 1985 +739
Continue to review full report at Codecov.
|
landonreed
left a comment
landonreed
left a comment
There was a problem hiding this comment.
LGTM. Although one thing I noticed was that maybe this field width could be a little narrower. This screenshot shows the field with the width changed to 120, which I think might be sufficient given that trip_short_name is typically reserved for things like vehicle numbers (according to the spec).
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
Let's find a better value for the placeholder for Trip Short Name.
|
Re @binh-dam-ibigroup's comments: I found the trip_short_name field at this example: https://support.google.com/transitpartners/answer/6377422?hl=en. The GTFS spec outlines trip_short_name as "Public facing text used to identify the trip to riders, for instance, to identify train numbers for commuter rail trips. If riders do not commonly rely on trip names, leave this field empty. A trip_short_name value, if provided, should uniquely identify a trip within a service day; it should not be used for destination names or limited/express designations." As @landonreed said, it's mostly reserved for vehicle numbers so perhaps '801' doesn't represent a great example. The MBTA uses three digits such as '005' for their commuter rail trip_short_names - perhaps this would be a better example to use. Let me know and I will update. |
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
Looks good otherwise!
…idth to 120 Changed trip_short_name width to 120 from 200 of previous commit Close #569
|
@landonreed, I updated the width here to be 120 for this and kept the 801 placeholder. Let me know if any further changes are needed. |
@philip-cline FYI, the markdown way to add checks is to write it as Example:
|
Checklist
devbefore they can be merged tomaster)Description
Added a trip_short_name field (Closes #569 ) to the timetable editor (by adding object in timetable.js). Also added tests to end-to-end.js to enter a value in trip_short_name.