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 4 hours ago | parent [-]

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.

▲

kps 3 hours ago | parent [-]

> 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 2 hours ago \| parent [-]
		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?????