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
-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
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.
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.
In many environments that depends on more than just code review, e.g. CI.
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?????
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.
So you could require `Verified+2` (CI), `Code-Review+2`, and `Design+2`, for example.
[0]: <https://gerrit-review.googlesource.com/Documentation/config-...>
The nuance is comments on the PR itself, rather than the state of the approval, which is binary (or ternary, if you want to count leaving it in an unknown state for extended periods of time).
Someone else knows the other portion well and sees the +1 and decides to +2.
In practice this ends the stalemate where partial owners don't feel confident to approve the whole thing
Having several people review each separate parts but not understanding the others' can cause interaction bugs. If such bugs cannot happen (say, due to modularity, or type safety guarantees etc), then it won't be the case where you need to have a partial approve.
I am not a fan of partial approve. Either you think the code is approvable, or it isn't.
You can require approvals from N domains from (potentially) different people.
Some people think that PR status can also communicate rationales and partial approvals.
Some think that should be done with tags and comments.
Lots of request systems have multiple stages between "open" and "resolved".
But with this, a non-maintainer could review be allowed to give a +1 or -1, but not a -2 or +2, and it is more clear that a "+1" isn't sufficient for actually merging the PR.
To run the process smoothly, one can just hope that the team/tech lead is an ideal developer. Otherwise they are in a position where no one senior than them is available for the code review and any one junior would just rubber stamp their PR's.
1. Is the PR suitable, and therefore should be approved, and
2. Is this person suitable to make that decision.
If 2 is false then the person should remove themselves from the list of reviewers. Then 1 can follow its normal process.
Also:
> If [a person is not suitable to make the decision of whether the PR should be approved] then the person should remove themselves from the list of reviewers.
This doesn't reflect what sometimes happens in real life. Someone could have sufficient specialized knowledge to be able to veto a PR, without having sufficient broader knowledge to approve a PR. That person should definitely be left on the reviewer list, with the ability to veto, the necessity to remark if he has vetoed or not, and the inability to definitively approve.
It is necessary for this specialist to notate "I have finished examining this PR, and there is nothing within my expertise that would cause me to veto it" before the PR is advanced.
Unfortunately, in a binary system, that often equates to him having to say "I approve" even though this does not truly capture the intent. Then you wind up with hacky work-arounds, like requiring a minimum number of approvals.