Exclude author from gerrit review

Question

I want to disallow the author of a change to review his/her own changes in gerrit. I'm aware of this suggested hack, but that doesn't really solve the issue.

Now I learned from the gerrit issues that gerrit's hardcoded rules can be modified by custom prolog code, so it should potentially be possible to modify the workflow as I want. However, I have never modified gerrit's workflow before and I don't know much prolog.

Does anyone have a small working example of custom rules for gerrit using this prolog engine?

I will happily accept other alternatives of how to forbid authors doing a self-review, given they do not require my team to change the current workflow.

Uncharitable

Answer 1

I found a very easy answer in this google group: groups.google.com/disable-self-review

In a parent project (default is the All-Projects project) add this to the project.config file in refs/meta/config branch:

[access "refs/*"]
label-Code-Review = block -2..+2 group Change Owner

and in the groups file in the same branch add this line

global:Change-Owner Change Owner

Then take the statement that allows the right into the child project's project.config:

label-Code-Review = -2..+2 group Developers

Make sure to do write the block statement in the parent project and give the rights in the child project. If allow and block are in the same file the allow will overrule (See this). This will block the owner of a change to give -2 or +2. It will leave the -1 and +1 options intact. You can add a similar statement for any other custom labels you might use.

Anthrax

Where the statement label-Code-Review = -2..+2 group Developers should be present in child project.config? Under which heading? – Garrek

just add deny -2 +2 for Change Owner in label Code Review works for me. – Unlovely

It works for me. Do you have any idea how to config if I want to allow one or more owners to review their own changes? – Chuchuah

@Chuchuah Should work by adding "label-Code-Review = -2..+2 name" in the parent project. But keep in mind that it is good practice to have someone else review your code no matter how experienced you are. – Anthrax

@Anthrax thank you! I replace name with a group which contains the list of excluded users and it works. And thanks for your advice. The excluded users are bot users for special tasks. – Chuchuah

Answer 2

I'm not sure that this what you are looking for but it might give you some inspiration. According to this discussion the following fragment approves changes only if the reviewer and the change owner are not the same person.

  % If a reviewer approved the change, its OK.
  submit_rule(submit(CR)) :-
    change_owner(Owner),
    max_with_block('Code-Review', -2, 2, ok(Reviewer)),
    not_same(Owner, Reviewer),
    CR = label('Code-Review', ok(Reviewer)),
    !.

Nucleolated

Answer 3

If you haven't found it already, here is the official description on how to do this using prolog:

https://gerrit-review.googlesource.com/Documentation/prolog-cookbook.html#_example_8_make_change_submittable_only_if_tt_code_review_2_tt_is_given_by_a_non_author

Cubiculum

I have not setup gerrit using git. I have followed more or less this guide [digitalocean.com/community/tutorials/…. Where do I find rules.pl in that case? Where would rules.pl go in that case? – Skellum

Answer 4

I posted this answer to the question that you linked to, but it may lead you in the right direction:

I wrote 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)

Irrationality

Which location I should keep this rule.pl? – Garrek

@ShashiRanjan the rule goes in the refs/meta/config branch for whatever project you want it to apply to (more info in the docs). Since I wanted it to apply to all projects in my Gerrit instance, I put it in the refs/meta/config branch of the All-Projects project – Irrationality

@hairraisin, any idea how i can combine 2 submit_filters in 1 rules.pl file? – Festival

Answer 5

The below prolog code will bypass the user/admin account from self-review blockage and block all other commit authors for self-review in the Gerrit repo. This code worked for me.

submit_rule(S) :-
    gerrit:default_submit(X),
    X =.. [submit | Ls],
    add_non_author_approval(Ls, R),
    S =.. [submit | R].
add_non_author_approval(S1, S2) :-
    C = user(000000),
    gerrit:commit_author(C),
    gerrit:commit_label(label('Code-Review', 2), R),
    R == C, !,
    S2 = [label('Non-Author-Code-Review', ok(C)) | S1].
add_non_author_approval(S1, S2) :-
    gerrit:commit_author(A),
    gerrit:commit_label(label('Code-Review', 2), R),
    R \= A, !,
    S2 = [label('Non-Author-Code-Review', ok(A)) | S1].
add_non_author_approval(S1, [label('Non-Author-Code-Review', need(_)) | S1]).

Limicolous

C = user(000000) here instead 000000 add Gerrit user ID of the account to bypass from the self review – Limicolous