[llvm-branch-commits] [lld] ELF: CFI jump table relaxation. (PR #147424)

[Fangrui Song via llvm-branch-commits]

MaskRay wrote:

> > So just 'Requested changes' should be enough to declare that your approval is needed?
> 
> Also "Redy to merge" is green even I remove my approval or request changes?

You're right that a PR is "ready to merge" without any approval and dismissal doesn't gate merge technically. The reason I will use it on lld/ELF and llvm/lib/MC changes is the approval state is very clear but the "Requested changes" state less prominent, and easily dismissed by "re-request review".
For changes of meaningful scope in areas I maintain, I'd rather the at-a-glance state reflect that domain review is still pending.

Fair point that dismissal can read as "please re-review".
That's not the intent.

If part of the friction is that your dismissal is auto-putting the PR back on your review-requested list, you can leave a comment without approving the PR, then this PR will be removed from your review-requested list.


https://github.com/llvm/llvm-project/pull/147424