-
Notifications
You must be signed in to change notification settings - Fork 28
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
bundle analysis: fix unknown asset size compute #914
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #914 +/- ##
=======================================
Coverage 96.25% 96.25%
=======================================
Files 823 823
Lines 18999 19001 +2
=======================================
+ Hits 18288 18290 +2
Misses 711 711
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
measurables_to_fetch = [ | ||
item for item in list(BundleAnalysisMeasurementsAssetType) | ||
] | ||
elif ASSET_TYPE_UNKNOWN in asset_types: | ||
measurables_to_fetch = [ | ||
BundleAnalysisMeasurementsAssetType.REPORT_SIZE, |
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.
I saw ASSET_SIZE is another variable on this type; jw why its not included in this list?
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.
ASSET_SIZE is for tracking the size of individual javascript assets, so the sum of all asset measurements equals to the JAVASCRIPT_SIZE. And all the other types are aggregate types, notably REPORT_SIZE is the sum of all assets.
So we can compute the trends line of unknown asset sizes by doing a computation of:
UNKNOWN_SIZE = REPORT_SIZE - JS_SIZE - STYLESHEET_SIZE - FONT_SIZE - IMAGE_SIZE
and
ASSET_SIZE can be ignored because JS_SIZE = sum(ASSET_SIZE)
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.
ahhhh okay makes a lot of sense! Thanks
If looking to get unknown asset type in the filters only retrieve the necessary known asset types to compute it
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.