-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace location groups with stop areas #375
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works well for me testing against the front end PR. A couple questions, mostly for my info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one quick fix for a bug around pattern names
Co-authored-by: Philip Cline <[email protected]>
…reflect changes suggested in PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me. Thanks for the fixes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow this is a heavy set of changes.
Just a few small questions but overall thanks for this! This does not look like it was easy work.
return locationGroupById.get(haltId).location_group_name; | ||
} else if (stopAreaById.containsKey(haltId)) { | ||
Area area = areaById.get(haltId); | ||
return area.area_name != null ? area.area_name : area.area_id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we return the id as a fallback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@philip-cline Does the UI require the area_id? If not, can the default fromTerminusNameUnknown
or toTerminusNameUnknown
value be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI doesn't require the area_id but I think it looks a lot better while we work on implementing area names.
… is not available
Checklist
dev
before they can be merged tomaster
)Description
Deprecated location groups and replace with stop areas and areas.