Monorail Project: gerrit Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 56 users
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 Back to list
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 (was: NULL)
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
Sign in to add a comment