amp-github-apps icon indicating copy to clipboard operation
amp-github-apps copied to clipboard

bundle-size: Allow using past approval for minor changes

Open jridgewell opened this issue 4 years ago • 4 comments

Eg, https://github.com/ampproject/amphtml/pull/28806#pullrequestreview-429426897

Will approved the CL, but I needed to make a minor change to satisfy the type checker. It's a little annoying to have to request a re-review for such small changes.

jridgewell avatar Jun 12 '20 04:06 jridgewell

This would require a much bigger change to the bot than I can handle, relative to the payoff (i.e., I think of it as a P3 that would require a P1 level of work...)

To make this happen, the bot will have to be changed so that when a commit adds more than the allowed bundle-size and was approved earlier, then it has to check whether it was already approved by someone (which it doesn't do), check whether the new change is significant compared to the previously approved commit (which means it'll have to store not only each individual file's bundle size per commit, but also which commit received the approval), and determine whether it requires re-approval

It's a little annoying to have to request a re-review for such small changes.

Is the annoyance big enough to merit a couple weeks of work on this? :)

danielrozenberg avatar Jun 19 '20 18:06 danielrozenberg

@jridgewell fixit week is a perfect time to answer that question ^

danielrozenberg avatar Feb 22 '21 17:02 danielrozenberg

Couple of weeks? Probably not worth it. But if it could be done in a fixit, that would be awesome.

jridgewell avatar Feb 22 '21 19:02 jridgewell

I'll push work on this for the end of this week and see if I can figure out a middle ground or at least make some initial progress on this

danielrozenberg avatar Feb 22 '21 20:02 danielrozenberg