alt Hacker News> PR approval is too boolean. The PR is approved or it's not approved. Real code review, like real life, lives in the middle
This is have-your-cake-and-eat-it. PR approval is a permission so is a boolean. Of course it is. Either the code can be merged or it can't.
What's being described really here is just something to make you feel slightly better about yourself whilst approving code you hate ("we should revisit this..."). Just open a new ticket.
Replies
I mean, that’s fair no? If the UX creates an impasse for the user then this leads to friction in the process. There’s more than one way to address it. One is the user overcomes his own internal dilemma, the other is the UX helps him get there. For example, would be cool if there was a way to do a conditional approval with an issue tied to a stacked PR or something similar (just throwing ideas, point is to surface up the friction as a UX take not a protocol or API issue with git)
This exists in Azure DevOps as Approve with Suggestions
Gerrit has -2...+2.
-2: This is a bad idea, don't do that
-1: This is a good idea but needs improvement
+1: LGTM but I don't have enough knowledge or authority to approve
+2: Approved