ref: Only display bundle and patch data on PullsTable #2934
Conversation
nicholas-codecov
force-pushed
the
gh-eng-1497-updates-to-pulls-table
branch
from
267d97f to
f8119d6
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #2934 +/- ##
==========================================
+ Coverage 98.37% 98.39% +0.01%
==========================================
Files 885 877 -8
Lines 13062 12931 -131
Branches 3481 3384 -97
==========================================
- Hits 12850 12723 -127
+ Misses 208 204 -4
Partials 4 4
... and 11 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #2934 +/- ##
==========================================
- Coverage 98.37% 98.35% -0.03%
==========================================
Files 885 880 -5
Lines 13062 12972 -90
Branches 3481 3454 -27
==========================================
- Hits 12850 12758 -92
- Misses 208 210 +2
Partials 4 4
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #2934 +/- ##
==========================================
- Coverage 98.37% 98.35% -0.03%
==========================================
Files 885 880 -5
Lines 13062 12972 -90
Branches 3463 3436 -27
==========================================
- Hits 12850 12758 -92
- Misses 208 210 +2
Partials 4 4
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Bundle ReportChanges will decrease total bundle size by 22.8kB ⬇️
|
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 #2934 +/- ##
================================================
- Coverage 98.37000 98.35000 -0.02000
================================================
Files 885 880 -5
Lines 13062 12972 -90
Branches 3463 3454 -9
================================================
- Hits 12850 12758 -92
- Misses 208 210 +2
Partials 4 4
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Bundle ReportChanges will decrease total bundle size by 22.8kB ⬇️
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
| render(<PullsTable />, { wrapper: wrapper(queryClient) }) | ||
|
|
||
| const bundleAnalysis = await screen.findByText('Upload: ✅') | ||
| const bundleAnalysis = await screen.findByText('Upload: ⏳') |
There was a problem hiding this comment.
is the default this hourglass icon now?
There was a problem hiding this comment.
No, not the default value. It's just reflective of the API response that's being mocked.
| patch = pull?.compareWithBase?.patchTotals?.percentCovered ?? 0 | ||
| change = pull?.compareWithBase?.changeCoverage ?? 0 | ||
| let patch = <p>-</p> | ||
| if (pull?.head?.coverageStatus === 'ERROR') { |
There was a problem hiding this comment.
There an enum for these? 'ERROR', 'PENDING' and 'COMPLETED'
There was a problem hiding this comment.
Don't really need it, the coverageStatus type is a union of string literals ERROR | PENDING | COMPLETED, so it'll error and fail the build if you change the return type. Also using strings directly like this is overall better for compression and smaller bundle sizes
There was a problem hiding this comment.
overall better for compression and smaller bundle sizes
If you have a source on this, would love to read more about the magnitude of gains here.
with respect to it erroring on the build by changing the return type, I'm sure there is a way to update the zod schema to account for this type on either end even with an enum.
In any case, just a thought that may improve readability for some
There was a problem hiding this comment.
Here's an example PR of swapping to regular strings from the object style enum I believe you're talking about, and the savings in size: getsentry/sentry-javascript#4280.
A different concern with using the object syntax like we do in Gazebo (which I think we need to iterate on, and move away from) is that the field names themselves can not be minified. There are also issues with tree shaking that come up since we code split quite heavily so this also brings up issues with bundle sizes.
An alternative I'd be alright exploring would be something more along the lines of:
export const COMMIT_STATUS_COMPLETED = 'COMPLETED'
export const COMMIT_STATUS_ERROR = 'ERROR'
export const COMMIT_STATUS_PENDING = 'PENDING'
const CommitStatusSchema = z.union([
z.literal(COMMIT_STATUS_COMPLETED),
z.literal(COMMIT_STATUS_ERROR),
z.literal(COMMIT_STATUS_PENDING),
])There was a problem hiding this comment.
Very neat! 2% savings definitely adds up over time, thanks for sharing that.
I think having a shared schema like that solves my original concern which is the improved readability, and we won't need the object syntax either. I'm all for it
| bundleAnalysis = <>Upload: ❌</> | ||
| const change = | ||
| pull?.bundleAnalysisCompareWithBase?.bundleChange?.size.uncompress | ||
| const content = `${change > 0 ? '+' : ''}${formatSizeToString(change)}` |
There was a problem hiding this comment.
if the change < 0, should we show a "-" sign?
There was a problem hiding this comment.
by default as a negative number the - is already present
There was a problem hiding this comment.
ah okay makes sense, I wasn't sure since the "formatSizeToString" was taking the abs of the number
Looking at your screenshot that seems to be the case, just a little hard to parse atm
There was a problem hiding this comment.
Ahh, no worries. formatSizeToString only abs the size to for the size checks to see if we can concatenate to a larger size (kB, MB, etc.) we than use the passed value directly in the format, rather than the absolute'ed value.
nicholas-codecov
force-pushed
the
gh-eng-1497-updates-to-pulls-table
branch
from
858eeeb to
f40908d
Compare
Description
This PR cleans up the current pulls table removing the project coverage, coverage from base columns, and team versions of these tables. This PR also updates the patch and bundle columns to render their status:
COMPLETED,ERROR, orPENDINGto better inform the user.codecov/engineering-team#1497
Notable Changes
Screenshots
Before:
After: