-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix: mark riverside as a terminal station #784
Conversation
@@ -5757,7 +5757,7 @@ | |||
"Green-D" | |||
], | |||
"platform": null, | |||
"terminal": false, | |||
"terminal": true, | |||
"announce_arriving": false, | |||
"announce_boarding": true | |||
} |
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.
If helpful for reference
Coverage of commit
|
@sloanelybutsurely my fault for not specifying this in the ticket but could we ensure that this also gets fixed (or maybe was never broken) at Medford/Tufts and Union Square, in addition to Riverside? Those would be the complete set of terminals that have PA/ESS. |
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.
Huh, it looks like this was just an oversight from when real-time data was originally enabled for these signs (a long time ago!), and maybe we never noticed because we were rarely able to make departure predictions at this terminal. Pending @dmaltzan's comment it looks fine to me.
For reproducing the issue, it might help to reach out to Firas since he was originally able to do so. I'm also curious about exactly how it was done, since I haven't used Glides for testing before. |
Since this is a very low-risk change, and pretty evidently correct by inspection, I think it would be fine to merge as-is. Definitely feel free to verify end-to-end if you'd like, though. |
It seems something was/is keeping predictions based on Glides data from being created. I've deployed to dev-green to let Firas verify this fixes the issue. |
Summary of changes
Asana Ticket: "BRD" showing on GL terminals too early
Sets
terminal: true
for Riverside so when a prediction is 0 stops awayBRD
is not shown until within the predicted departure time.Note: I still haven't reproduced the original issue locally. Any help verifying before merging is much appreciated.
Reviewer Checklist