Excluding author from peer reviewer list in gerrit

I'm setting up the access control for my company in gerrit and in our current internal process has cross-over between peer reviewers and coders (they tend to be the same group of people). We also want to only require 1 reviewer to peer revi开发者_运维技巧ew the code and submit it if it looks good.

With the default setup any user with the +2: Looks good to me, approved option can peer review their own code.

Is there any way to prevent the author from reviewing their own code, but still allow them to fully review other's code? I haven't been able to find any kind of exclude author in the access control group setup or permissions setups.

---

The Gerrit Cookbook Example 8 does not strictly prevent the Author to review his/her own change, but will require someone else to +2 it before being able to submit.

---

This is working for me, but it's a quick hack:

• allows a configurable number of +1s to count as a +2 for manual submission
• optionally automatically submit with enough +1 votes
• optionally counts -1 votes as countering +1 votes for the purposes of the tally
• optionally ignores the uploader's own +1 (you may prefer a check against the author, which I've not done)

I've tweaked my earlier answer so it doesn't assume you're using a mysql server.

You might want to move the logfile somewhere it'll be subject to any normal log rotation - perhaps in ../logs/comment-added.log.

I've tried to pull the configurable bits to the fore. Call this file comment-hook and put it in $gerrit_root/hooks, chmod it 755 or similar. Set up a robot user in the admin group so the hook can use the sql interface (and comment +2 on things with enough +1s).

#!/usr/bin/perl
#
# comment-hook for a +2 approval from a simple quorum of +1 votes.
#
# Licence: Public domain. All risk is yours; if it breaks, you get to keep both pieces.

$QUORUM = 2; # Total number of +1 votes causing a +2
$PLEBIANS = 'abs(value) < 2'; # or 'value = 1' to ignore -1 unvotes
$AUTO_SUBMIT_ON_QUORACY = '--submit'; # or '' for none
$AND_IGNORE_UPLOADER = 'and uploader_account_id != account_id'; # or '' to let uploaders votes count

$GERRIT_SSH_PORT = 29418;
$SSH_PRIVATE_KEY = '/home/gerrit2/.ssh/id_rsa';
$SSH_USER_IN_ADMIN_GROUP = 'devuser';

# Hopefully you shouldn't need to venture past here.

$SSH = "ssh -i $SSH_PRIVATE_KEY -p $GERRIT_SSH_PORT $SSH_USER_IN_ADMIN_GROUP\@localhost";

$LOG = "/home/gerrit2/hooks/log.comment-added";
open LOG, ">>$LOG" or die;

sub count_of_relevant_votes {
        # Total selected code review votes for this commit
        my $relevance = shift;
        $query = "
                select sum(value) from patch_sets, patch_set_approvals
                where patch_sets.change_id = patch_set_approvals.change_id
                and patch_sets.patch_set_id = patch_set_approvals.patch_set_id
                and revision = '$V{commit}'
                and category_id = 'CRVW'
                and $relevance
                $AND_IGNORE_UPLOADER
                ;";
        $command = "$SSH \"gerrit gsql -c \\\"$query\\\"\"";
        #print LOG "FOR... $command\n";
        @lines = qx($command);
        chomp @lines;
        #print LOG "GOT... ", join("//", @lines), "\n";
        # 0=headers 1=separators 2=data 3=count and timing.
        return $lines[2];
}

sub response {
        my $review = shift;
        return "$SSH 'gerrit review --project=\"$V{project}\" $review $V{commit}'";
}

# ######################
# Parse options

$key='';
while ( $_ = shift @ARGV ) {
        if (/^--(.*)/) {
                $key = $1;
        }
        else {
                $V{$key} .= " " if exists $V{$key};
                $V{$key} .= $_;
        }
}
#print LOG join("\n", map { "$_ = '$V{$_}'" } keys %V), "\n";

# ######################
# Ignore my own comments

$GATEKEEPER="::GATEKEEPER::";
if ($V{comment} =~ /$GATEKEEPER/) {
        # print LOG localtime() . "$V{commit}: Ignore $GATEKEEPER comments\n";
        exit 0;
}

# ######################
# Forbear to analyse anything already +2'd

$submittable = count_of_relevant_votes('value = 2');
if ($submittable > 0) {
        # print LOG "$V{commit} Already +2'd by someone or something.\n";
        exit 0;
}

# ######################
# Look for a consensus amongst qualified voters.

$plebicite = count_of_relevant_votes($PLEBIANS);

#if ($V{comment} =~ /TEST:(\d)/) {
#        $plebicite=$1;
#}

# ######################
# If there's a quorum, approve and submit.

if ( $plebicite >= $QUORUM ) {
        $and_submitting = ($AUTO_SUBMIT_ON_QUORACY ? " and submitting" : "");
        $review = " --code-review=+2 --message=\"$GATEKEEPER approving$and_submitting due to $plebicite total eligible votes\" $AUTO_SUBMIT_ON_QUORACY";
}
else {
        $review = " --code-review=0 --message=\"$GATEKEEPER ignoring $plebicite total eligible votes\"";
        # print LOG "$V{commit}: $review\n";

        exit 0;
}

$response = response($review);

print LOG "RUNNING: $response\n";
$output = qx( $response 2>&1   );
if ($output =~ /\S/) {
        print LOG "$V{commit}: output from commenting: $output";
        $response = response(" --message=\"During \Q$review\E: \Q$output\E\"");
        print LOG "WARNING: $response\n";
        $output = qx( $response 2>&1   );
        print LOG "ERROR: $output\n";
}

exit 0;

---

Gerrit allows you to set up prolog "submit rules" that define when a change is submittable.

The documentation includes several examples, including one that prevents the author from approving his own change.

---

You can do it from the GUI in the access tab. Go to the /refs/heads/ section -> add group 'change owner' in Label Code-Review section -> choose -1..+1

This will make the change owner to privilege for giving -1 to +1

---

I have just written this prolog filter for our Gerrit installation. I did it as a submit_filter in the parent project because I wanted it to apply to all projects in our system.

%filter to require all projects to have a code-reviewer other than the owner
submit_filter(In, Out) :-
    %unpack the submit rule into a list of code reviews
    In =.. [submit | Ls],
    %add the non-owner code review requiremet
    reject_self_review(Ls, R),
    %pack the list back up and return it (kinda)
    Out =.. [submit | R].

reject_self_review(S1, S2) :-
    %set O to be the change owner
    gerrit:change_owner(O),
    %find a +2 code review, if it exists, and set R to be the reviewer
    gerrit:commit_label(label('Code-Review', 2), R), 
    %if there is a +2 review from someone other than the owner, then the filter has no work to do, assign S2 to S1
    R \= O, !,
    %the cut (!) predicate prevents further rules from being consulted
    S2 = S1.
reject_self_review(S1, S2) :-
    %set O to be the change owner
    gerrit:change_owner(O),
    % find a +2 code review, if it exists, and set R to be the reviewer - comment sign was missing
    gerrit:commit_label(label('Code-Review', 2), R), 
    R = O, !,
    %if there isn't a +2 from someone else (above rule), and there is a +2 from the owner, reject with a self-reviewed label
    S2 = [label('Self-Reviewed', reject(O))|S1].
%if the above two rules didn't make it to the ! predicate, there aren't any +2s so let the default rules through unfiltered
reject_self_review(S1, S1).

The benefits (IMO) of this rule over rule #8 from the cookbook are:

• The Self-Reviewed label is only shown when the the change is being blocked, rather than adding a Non-Author-Code-Review label to every change
• By using reject(O) the rule causes the Self-Reviewed label to literally be a red flag
• As a submit_filter instead of a submit_rule, this rule is installed in a parent project and applies to all sub-projects

Please Note: This rule is authored to prevent the Owner from self-reviewing a change, while the example from the cookbook compares against the Author. Depending on your workflow, you may want to replace the 2 gerrit:change_owner(O) predicates with gerrit:commit_author(O) or gerrit:commit_committer(O)