Skip to content

Conversation

@hari45678
Copy link
Contributor

Which problem is this PR solving?

Description of the changes

This GH Workflow would go through the following steps:

  1. Calculate the actual bundle size by using build directory generated when we run npm run build
  2. Save that size to a new_bundle_size.txt file
  3. Now fetch the bundle_size.txt file from cache, if present.
  4. If cache hits, compare the bundle_size.txt (contains old size) with the new_bundle_size.txt (contains new size).
  5. If its more than 2%, fail the CI
  6. Else pass it.
  7. If cache miss, save the bundle_size.txt to cache
  8. Finally, if everything's alright, save the current bundle size file to cache. (but only on pushes to main branch)

How was this change tested?

Checklist

Signed-off-by: Hariom Gupta <[email protected]>
@hari45678 hari45678 requested a review from a team as a code owner January 11, 2025 12:19
@hari45678 hari45678 requested review from joe-elliott and removed request for a team January 11, 2025 12:19
@hari45678 hari45678 changed the title Create check_bundle.yml [ci]: add bundle_check workflow Jan 11, 2025
@codecov
Copy link

codecov bot commented Jan 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.58%. Comparing base (43d832b) to head (5af0b1b).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2586   +/-   ##
=======================================
  Coverage   96.58%   96.58%           
=======================================
  Files         255      255           
  Lines        7732     7732           
  Branches     2009     1996   -13     
=======================================
  Hits         7468     7468           
  Misses        264      264           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hari45678
Copy link
Contributor Author

Just noticed, we already do a build in Lint and Build workflow. Should I include the bundle_check part in that workflow instead of creating a separate one?

@yurishkuro
Copy link
Member

I think it's good to have this one separate, because if it fails we can occasionally override it and still merge (when we deem the size change legitimate), we probably won't make this check required.

@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label Jan 11, 2025
@yurishkuro
Copy link
Member

It doesn't look like your new workflow ran, perhaps a syntax error

@hari45678
Copy link
Contributor Author

It doesn't look like your new workflow ran, perhaps a syntax error

It did run https://github.com/jaegertracing/jaeger-ui/actions/runs/12724148042/job/35470007592?pr=2586

I would try fixing as per the comments though

Signed-off-by: Hariom Gupta <[email protected]>
Signed-off-by: Hariom Gupta <[email protected]>
@hari45678
Copy link
Contributor Author

hari45678 commented Jan 11, 2025

Looks like some issue. How is it getting a cache hit? We haven't yet pushed to main branch after merging this PR

Signed-off-by: Hariom Gupta <[email protected]>
yurishkuro
yurishkuro previously approved these changes Jan 11, 2025
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great!

Signed-off-by: Hariom Gupta <[email protected]>
@yurishkuro yurishkuro changed the title [ci]: add bundle_check workflow [ci]: Add workflow to guard against growing bundle size Jan 11, 2025
@hari45678
Copy link
Contributor Author

hari45678 commented Jan 11, 2025

There was an issue (cache was getting saved in the PRs) which I just fixed in the latest commit. Actually actions/cache restores the cache but also saves it as a post action. action/cache/restore doesn't do that step.

@yurishkuro yurishkuro merged commit bd9a770 into jaegertracing:main Jan 11, 2025
9 checks passed
@hari45678 hari45678 deleted the add_workflow branch January 17, 2025 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:ci Change related to continuous integration / testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add CI workflow to guard against increases in the bundle size

2 participants