search

preactjs / compressed-size-action

GitHub Action that adds compressed size changes to your PRs.
https://github.com/marketplace/actions/compressed-size-action
MIT License
603 stars 84 forks source link

Feature Request: Block PRs When Exceeding Size Thresholds #107

Open vmerino opened 4 months ago

vmerino commented 4 months ago

Hello,

Are there any plans to enable blocking the merge of a pull request if it exceeds a specified size threshold? Thanks for your time!

rschristian commented 4 months ago

Wouldn't this get quite tedious to constantly bump?

My gut says this is better left as information for people to check while reviewing than a pass/fail action.

vmerino commented 4 months ago

Hmm... maybe? Wouldn't it be pass by default, and fail only if threshold is exceeded? If the threshold is set too low, then yes it would be tedious. It seems like some could benefit from having a mechanism for egregious size violations though. For instance, we recently had a case where tree-shaking wasn't occurring as expected, and our bundle size ballooned by 10 MB. In extreme cases like that, I would prefer that the PR be automatically blocked.

rschristian commented 4 months ago

If the threshold is set too low, then yes it would be tedious.

Yeah, this is what I was referring to; I think it'd be quite difficult to have the bar be low enough that's it's often useful while not being a tedious pain.

For example, for a web app, you'd probably set that for 10kb-ish? But then if you need to add some graph lib, or some lib inlines SVGs/CSS/whatever, you'll be back in the action bumping up the size every time you're adding a sizable feature. To stop it from being a pain, you bump it the detection limit to say 1mb, but then that's not realistically useful, as it's only going to catch the most egregious bugs, so you might miss some regressions you wish you hadn't.

There's a lot of nuance to size increases and if they're justified or not which is why I'd say this is better left as a metric for a person to look over, personally.