feat: update bundle_analysis comment requirement #532
Conversation
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #532 +/- ##
==========================================
- Coverage 97.50% 97.50% -0.01%
==========================================
Files 420 420
Lines 35320 35360 +40
==========================================
+ Hits 34440 34478 +38
- Misses 880 882 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #532 +/- ##
==========================================
- Coverage 97.50% 97.50% -0.01%
==========================================
Files 420 420
Lines 35320 35360 +40
==========================================
+ Hits 34440 34478 +38
- Misses 880 882 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #532 +/- ##
==========================================
- Coverage 97.50% 97.50% -0.01%
==========================================
Files 420 420
Lines 35320 35360 +40
==========================================
+ Hits 34440 34478 +38
- Misses 880 882 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #532 +/- ##
==========================================
- Coverage 97.53% 97.52% -0.01%
==========================================
Files 451 451
Lines 36043 36083 +40
==========================================
+ Hits 35153 35191 +38
- Misses 890 892 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes This change has been scanned for critical changes. Learn more |
giovanni-guidini
changed the title
| and comparison.total_size_delta > changes_threshold | ||
| ) | ||
| case _: | ||
| log.warning( |
There was a problem hiding this comment.
This is technically dead code because the YAML validation should let it get this far.
I added this just so it wouldn't break the interface by returning None... maybe I should remove it? Even if it does return None it won't break the app.
There was a problem hiding this comment.
I think it's fine as long as you leave a comment explaining that this code should never be reached, (maybe even throw an exception? emit a metric? not sure, but we would need some way to observe that this does not happen)
| and comparison.total_size_delta > changes_threshold | ||
| ) | ||
| case _: | ||
| log.warning( |
There was a problem hiding this comment.
I think it's fine as long as you leave a comment explaining that this code should never be reached, (maybe even throw an exception? emit a metric? not sure, but we would need some way to observe that this does not happen)
giovanni-guidini
force-pushed
the
gio/bundle-comment-requirements-update
branch
from
c9e3e83
to
b3c671d
Compare
Updates the comment requirements for bundle_analysis. Respects the `comment.bundle_change_threshold` defined in the YAML and only notifies if changes are bigger than the threshold. (threshold is expected to be an int. So technically [this PR](codecov/shared#271) is a dependency of the present changes, because it does the coercion into an integer, as expected) Also adds the `"bundle_increase"` as a possible requirements. In this case on top of the threshold the change also needs to cause the bundle to _increase_ in size for a comment to be sent. closes: codecov/engineering-team#2021
giovanni-guidini
force-pushed
the
gio/bundle-comment-requirements-update
branch
from
b3c671d
to
619bdea
Compare
giovanni-guidini
force-pushed
the
gio/bundle-comment-requirements-update
branch
from
619bdea
to
2f69283
Compare
Updates the comment requirements for bundle_analysis.
Respects the
comment.bundle_change_thresholddefined in the YAMLand only notifies if changes are bigger than the threshold.
(threshold is expected to be an int. So technically this PR is a dependency of the present changes, because it does the
coercion into an integer, as expected)
Also adds the
"bundle_increase"as a possible requirements. In this caseon top of the threshold the change also needs to cause the bundle to increase
in size for a comment to be sent.
closes: codecov/engineering-team#2021
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.