Try adding bundle size github action #20305
Conversation
|
Size Change: 0 B Total Size: 734 kB ℹ️ View Unchanged
|
youknowriad
added
the
[Type] Automated Testing
|
Love it 😍 |
|
Neat, but it might get a bit "noisy" if this is adding comments to every PR? |
|
Noise is a valid concern but I think it's worth it. We can try and see. |
|
Cc @jsnajdr with his experience in Calypso repo for similar tool. |
|
Calypso has been using a similar tool for almost a year now. If the message is collapsed by default (doesn't post a wall of text that one needs to scroll over), and provides useful and actionable information, then it's not perceived as noise. The Calypso bot posts messages that are customized for Calypso and its architecture: grouping stats by entrypoints, async loaded sections (triggered by route navigation) and async loaded React components (triggered by a lazy React render). For Gutenberg, it could be useful to provide information about which packages got smaller, which ones got bigger, and also how the overall Gutenberg app was affected. I.e., how much has the size of JS and CSS that the user's browser needs to download before showing the block editor? |
|
Thanks for the insights @jsnajdr
That's exactly what the action is doing here
For this to be accurate, we're missing two things:
I wonder if the Github action I've used work for stylesheets too and I could just adapt the "pattern" to take these into consideration. |
|
Noting that I had a very similar task idea in the Project Management Automation project board:
I marked it as "Done" and linked it to this pull request as reference. |
|
I don't want to be annoying, bit I find this indeed a bit "noisy". I've not found myself looking at this at all (generally I can kind of guess how much code I'm adding or removing). This comment is especially annoying in email notifications, where it's a huge email (not collapsed) and I cannot unsubscribe from this kind of comments. It's often also tricked me into thinking someone replied to me PR, only to realise it was a bot! 🤖 Maybe there's a way to keep the information, but not as "proper" comment. Not sure if there can be something like pseudo comments, or appending the info to the PR description? |
|
It is indeed a bit noisy and not useful in all PRs, I do check it though for PRs that add npm packages, icons... I think appending to the description could work. |
|
@youknowriad also posting the comment only when there's a significant change to either direction could work. |
|
Wondering if this bot could be changed so that it reports as part of the checks process:
Possibly with a warning status if there is a "significant change" as @simison suggests? |
|
For what it's worth, I opened a pull request on the upstream action at preactjs/compressed-size-action#16 , including a bit of my own thoughts and a proposal for limiting its messaging. |
|
Hi all - good points about the comment being a little noisy. I've never come across anyone who has emails turned on for GitHub PR comments and totally didn't consider that when building the output. Regarding the use of checks - I'd actually attempted this previously but didn't pursue it. I just opened a draft PR with the changes, if anyone wants to give it a look: I'd be happy to merge this into |
|
Ah, I think Github action notifications can be turned off. https://github.com/settings/notifications |
|
Hey @developit Thanks for reaching out here and sorry for the delay to respond. @aduth do you have thoughts on the "checks" here? If I understand properly, checks will need a threshold to fail or succeed, for me, I'd prefer the bundle change to be more an "information" than a "status". The reasons being:
I don't think a threshold is something that can be unique across PRs. |
|
I'd tend to agree with you @youknowriad . I don't know that it's something we'd want to be a blocker for a pull request, even if it were possible to come to some reasonable determination of a threshold. It might be nice if checks supported some way to signal informational or advisory messaging. Via https://developer.github.com/v3/checks/runs/#update-a-check-run, here are the full set of "conclusion" options:
Neutral is probably the closest to something informational. But given the current UX, I'd still worry it would too easily be misinterpreted as a "problem" (i.e. problem = a lack of "all green"). Via https://developer.github.com/changes/2018-05-07-new-checks-api-public-beta/ : |
|
@aduth Interesting, I actually didn't know about "Neutral" checks. I'd be fine exploring it tbh if that option was available. |
Adds a Github action to compare the bundle size of the different packages between the PR branch and master.
Seems to work pretty well (see the comment here)