[wccrawford]

I was in camp 'boolean', but I think this has convinced me. I always had a problem that there were developers who didn't really understand the code, but would click 'approve' anyhow because they didn't see any problems in the parts they understood.

This meant that they were completely unable to actually 'approve' a review, but were only able to reject it. They were juniors, so they'd eventually get to that point, but by then, everyone would be used to just ignoring their approvals.

This provides that middle ground.

[hnfong]

Either the code gets merged or it does not. That's the inherent boolean part.

Given that, what's wrong with simply commenting on the PR to document the concerns, issues, lack of knowledge, etc?

Unless you're using those +/-2 to achieve some sort of goal... but you can also do that with labels, tags, etc. on the PR.

[m463]

I haven't used this kind of +/-2 but I think it might be good communication between reviewers.

sometimes I review something and say "approved", but sometimes I can only review part of it, and really need someone else to check what's out of my wheelhouse.

sort of "partially approved".

I have used systems that can set things like "requires 2 reviewers" or "bob, fred are required reviewers, elon and sam are optional reviewers".

also we had "thumbs up, thumbs down, and some comments might have a "task" associated with them as a required fix before approval"

optionally, maybe before you say "approved" you have an overall comment, and see the comments of other reviewers.

[kps]

> Either the code gets merged or it does not. That's the inherent boolean part.

In many environments that depends on more than just code review, e.g. CI.

[hnfong]

Sure, it depends more than code review, but the code review is still a boolean flag, i.e. BOOLEAN getsMerged = codeReview && passesCI && passesTests....

Unless you're implying codeReview is a score and a low code review score can be offset by higher scores elsewhere eg. passes more tests?????