GitHub PR review statuses in plain English:
Approve: Merge it when you think it's ready.
Comment: After you incorporate my feedback, someone should look at it again before merging.
Request changes: Do not merge this until I, SPECIFICALLY look at this again. I do not care if other people think it's good to go, it needs my blessing, and mine only, and I will fight you if you try to merge.
@VincentTunru Interesting, I thought of it as:
- Approve: Ready to merge.
- Comment: Ready to merge, but I've got some optional feedback or suggestions for improvements.
- Request changes: Not ready to merge.
Also, it's always frustrating when I have different required/optional feedback; but can only have one review status :/. It'd be nice to have it by individual comments.
And what's worse: at some point you internalise what the actual effect of these options are, and then "Request changes" suddenly feels really passive-aggressive, even though it was absolutely not intended that way by the reviewer.
@VincentTunru @noeldemartin 100%! I would only request changes in very specific cases, and most times only comment not to become a bottleneck. And I've been unfairly annoyed at people who requested changes in complete good faith ^^
@noeldemartin Yeah, that's what the labels seem to imply, but if you have mandatory approval turned on, then my descriptions are what actually happens: GitHub will prevent you from merging PRs that a contributor has "requested changes" on, even if someone else has approved it, and will block you from merging if you only have "comments".
Very frustrating when people have interpret them like you do, but that that then results in what I described.