-
Notifications
You must be signed in to change notification settings - Fork 66
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
Refactor all special case shield definitions inline #1081
Conversation
Do we know why the ShieldJSON got smaller in the stats? Just want to make sure we're not losing anything in the refactor. |
I changed |
I actually misspoke, the stats say the ShieldJSON is 1,484 bytes larger. That sounds like there's somehow an extra shield entry in this. I went shield by shield and couldn't find it though. |
I ran a script to compare the two shield json files between this PR and main, and the attached report shows the differences (in theory). |
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.
On review of the comparison report, the extra size is due to bannered Georgia routes. After this consolidation, the bannered Georgia routes also gained the overrideByRef definitions for routes 515 and 520 (the blue and green routes). The prior code had the overrides get set later in the file and only on the main Georgia route networks, not the variants. Once #1073 is complete, this bit of extra duplication will go away. As such, I think this is OK to proceed with!
Corollary to #1074. This PR removes the special case section at the bottom of
shield_defs.js
, and collates all the instances ofoverrideByRef
,overrideByName
andrefsByName
inline with their geographically sorted definitions.