Bundle Analysis: Routing report and comparisons #447
Conversation
shared/bundle_analysis/report.py
Outdated
| self.db_path = db_path | ||
| self.data = data | ||
|
|
||
| def get_size(self) -> Dict[str, int]: |
There was a problem hiding this comment.
Thoughts on renaming like get_sizes_by_route or something to represent that you are fetching a map of routeToSize instead of just 1 size? Or like get_sizes?
Mentioning because I did a double take in above when something singular sounding was returning something plural
base_sizes = self.base_report.get_size()
shared/bundle_analysis/report.py
Outdated
| results[route] = sum([asset.size for asset in asset_reports]) | ||
| return results | ||
|
|
||
| def get_size_by_route(self, route: str) -> Optional[int]: |
There was a problem hiding this comment.
nit: can consider dropping "by_route" in the function name since the argument route already communicates that context
but not sure if that's just a python thing where input args aren't so obvious so we need to help humans more
| imported Assets into the belonging route. Also this function returns a | ||
| BundleRouteReport object as this will be used for comparison and additional | ||
| data manipulation. | ||
| """ |
shared/bundle_analysis/comparison.py
Outdated
| all_routes, results = base_sizes.keys() | head_sizes.keys(), [] | ||
| for route_name in all_routes: | ||
| # Added new route | ||
| if route_name not in base_sizes: |
There was a problem hiding this comment.
Do we need to add the case where base_sizes[route_name] is 0 to avoid a potential divide by 0 issue at line 240? Can 0 even happen - maybe with rounding?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #447 +/- ##
==========================================
- Coverage 90.66% 90.01% -0.66%
==========================================
Files 397 324 -73
Lines 12230 9300 -2930
Branches 2069 1663 -406
==========================================
- Hits 11088 8371 -2717
+ Misses 1046 865 -181
+ Partials 96 64 -32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
closes: codecov/engineering-team#3032
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.