Skip to content
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-size delta check/message on PR #501

Open
LesnyRumcajs opened this issue Jul 20, 2022 · 3 comments · May be fixed by #937
Open

Bundle-size delta check/message on PR #501

LesnyRumcajs opened this issue Jul 20, 2022 · 3 comments · May be fixed by #937
Assignees
Labels

Comments

@LesnyRumcajs
Copy link
Contributor

LesnyRumcajs commented Jul 20, 2022

Based on a discussion with @anorth in #497 it would be beneficial to include the bundle size delta between master and candidate branch in a PR message.

Some changes are clearly increasing bundle size, but some of them are a bit sneaky (e.g. bumping a dependency that by default would now include a large set of unused features). It would be great to not have to have it at the back of the head but right in front of us.

I'd imagine a GH bot posting a message to PR (or modifying an existing one to not bloat the discussion) with a simple cargo clean && cargo build && make bundle && du -b output/builtin-actors.car on both branches, calculating the delta in bytes and %.

Probably something along the lines of this workflow would do the trick (that is to say, having a modifiable message with shell output): https:/LesnyRumcajs/grpc_bench/blob/master/.github/workflows/issue_bench.yml

@anorth
Copy link
Member

anorth commented Jul 20, 2022

Thanks!

An alternative implementation is a build step that writes the output size to a file that's checked in to git (orchestrate from Makefile or similar). In CI, build should check that the calculated value equals the written value. This puts the delta into code review directly rather than a comment on the GitHub issue, and makes the history of sizes easy to see.

Either could work,

@jennijuju jennijuju added this to the Network v17 milestone Aug 11, 2022
@anorth anorth added the good first issue Good for newcomers label Aug 11, 2022
@jennijuju
Copy link
Member

@LesnyRumcajs would you be open to take a stab on this one?

@LesnyRumcajs
Copy link
Contributor Author

@jennijuju Sure thing, I'll look into this next week.

@LesnyRumcajs LesnyRumcajs self-assigned this Dec 7, 2022
@jennijuju jennijuju removed this from the Network v17 milestone Dec 7, 2022
@LesnyRumcajs LesnyRumcajs linked a pull request Dec 12, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants