New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 308 link

Starred by 55 users

Issue metadata

Status: Released
Owner: ----
Closed: Oct 2012
Cc:

Blocked on:
issue 112
issue 288



Sign in to add a comment

Allow forbid of self +1 reviews

Reported by dav...@codeaurora.org, Oct 22 2009

Issue description

It would be nice to be able to configure Gerrit to forbid self +1 reviews.
Perhaps this could be done such that only people with +2 approval
permission could give themselves a +1 on a review.

We occasionally see code submissions where the author gives themselves a
positive review, and the approver has to notice and then ignore this
review.

 

Comment 1 by nasser@chromium.org, Oct 22 2009

Summary: Allow forbid of self +1 reviews

Comment 2 by s...@google.com, Oct 29 2009

Blockedon: 112
Status: Accepted
This is related to  issue 112  where we want to ensure verifier != code reviewer, or 
that neither is author, depending on site/project approval rules.

Comment 3 by j...@google.com, Nov 4 2009

On the other hand I routinely give "myself" +1 or even +2 in situations where I'm the
owner (but not the author) or a change, i.e. in cases where I've created a commit on
behalf of someone else and review that other person's change.

Comment 4 by s...@google.com, Nov 4 2009

I also tend to self-grant +1/+2 often.

Really this needs to be:

- per project.  Not every project wants to forbid self reviews.

- per review level.  Some projects might allow self Verified +1 but not Code Review.

- allow owners to bypass forbid.  Some projects might want to allow owners to self 
approve no matter what.

- forbid owners to self approve.  Some projects might want an owner to get approval 
from at least one other before submitting (so counter to the rule above).

There's a reason why we haven't implemented this yet, everyone wants a different 
function here.

Comment 5 by di...@google.com, Jan 25 2010

For what's worth we need something similar but in our case to not allow either +1 nor +2 when the reviewer 
matches the author (as jbq says the committer is not important). At least that's our use case.

Comment 6 by di...@google.com, Jan 26 2010

Actually since the Author can be filled in (spoofed) and the committer is the only piece of information trusted 
(assuming we trust Gerrit which I think it's the one generating the committer header) then if this feature is 
implemented it should be possible to restrict it when committers match reviewers too not only authors.

Comment 7 by sop@google.com, Jan 26 2010

Author can be spoofed by the client.  There is no verification of it.

Committer is supplied by the client, but Gerrit Code Review enforces
that the email address listed as the committer matches one of the
verified email addresses for the user who is uploading the commit.
Any attempt to spoof the committer email is rejected with an error.

Therefore, we can match committer to an account, and use that to
block a "self approval".

Comment 8 by sop@google.com, Jan 31 2010

Labels: Milestone-2.1.3

Comment 9 by sop@google.com, Jun 16 2010

Labels: -Milestone-2.1.3

Comment 10 by sop@google.com, Jun 16 2010

Blockedon: 288
This should be based on the other approval rules stuff
that  issue 288  hopes to provide.
Someone could always self verify their patches and merge directly to the mainline.
Project Member

Comment 12 by bklar...@gmail.com, Nov 10 2011

 Issue 1178  has been merged into this issue.
We should just be forbidden to +1 our own patchset. We should still be able to +1 a patchsets submitted by other people on a change we own.
Project Member

Comment 14 by bklar...@gmail.com, Apr 23 2012

Cc: sop@google.com
I think this is now possible with the prolog engine?  If so, maybe we just need to document how and then close this issue?

Comment 15 by jbjo...@gmail.com, Oct 24 2012

This is done and properly documented in the prolog-cookbook (example 8)

Project Member

Comment 16 by bklar...@gmail.com, Oct 24 2012

Labels: FixedIn-2.5
Status: Released

Comment 17 Deleted

Sign in to add a comment