Skip to content
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

chore: Adjust Harvard upper busway config with new child stops/routes. #824

Merged
merged 2 commits into from
Oct 3, 2024

Conversation

cmaddox5
Copy link
Contributor

@cmaddox5 cmaddox5 commented Oct 2, 2024

Summary of changes

Asana Ticket: Fix up Harvard busway berth assignments

Updated Harvard upper busway config to account for new berth assignments.

Reviewer Checklist

  • Meets ticket's acceptance criteria
  • Any new or changed functions have typespecs
  • Tests were added for any new functionality (don't just rely on Codecov)
  • signs.json changes were also made in signs_ui
  • This branch was deployed to the staging environment and is currently running with no unexpected increase in warnings, and no errors or crashes (compare on Splunk: staging vs. prod)

@cmaddox5 cmaddox5 requested a review from a team as a code owner October 2, 2024 14:42
Copy link
Collaborator

@panentheos panentheos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gist of this change looks correct, but the overall shape of this config looks incorrect, coming out of #809 . That PR added several new config entries, but they should probably have been added as additional source entries instead. Each config generates a separate row on the sign, so we typically want to collapse things by route, so each route gets one line.

We should fix this up so there are 9 configs, and they can have multiple sources if we need to e.g. support multiple stop ids for a route.

@cmaddox5
Copy link
Contributor Author

cmaddox5 commented Oct 3, 2024

The gist of this change looks correct, but the overall shape of this config looks incorrect, coming out of #809 . That PR added several new config entries, but they should probably have been added as additional source entries instead. Each config generates a separate row on the sign, so we typically want to collapse things by route, so each route gets one line.

We should fix this up so there are 9 configs, and they can have multiple sources if we need to e.g. support multiple stop ids for a route.

Ahh I see. That makes sense.

@cmaddox5 cmaddox5 requested a review from panentheos October 3, 2024 14:58
Copy link
Collaborator

@panentheos panentheos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@cmaddox5 cmaddox5 merged commit 8084379 into main Oct 3, 2024
2 checks passed
@cmaddox5 cmaddox5 deleted the cm/adjust-harvard-upper-busway branch October 3, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants