Restrict Submitter

48 views
Skip to first unread message

santhosh K

unread,
Dec 24, 2019, 2:08:00 PM12/24/19
to Review Board Community
Hi 

Where do we have to change for not allowing submitter to be reviewer?


David Trowbridge

unread,
Dec 24, 2019, 2:10:59 PM12/24/19
to reviewboard
Hi,

This is not something that is currently possible, for several reasons.

We allow people to assign to themselves because in our experience, people often like to create and iterate on drafts privately before publishing to their peers. If they create it and assign it to themselves, they can do that, and then change the reviewers once they're ready for feedback.

We allow people to leave reviews on their own changes because we've seen often that it's useful for explanations. One thing I'll often do is publish and then find that there's some issue, and I'll leave a review on my own change telling people to hold off until I can update it.

David

On Tue, Dec 24, 2019 at 12:08 PM santhosh K <keshw...@gmail.com> wrote:
Hi 

Where do we have to change for not allowing submitter to be reviewer?


--
Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
---
You received this message because you are subscribed to the Google Groups "Review Board Community" group.
To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard/503472cc-4bfb-49c3-9c8c-3d319b9847a3%40googlegroups.com.

santhosh K

unread,
Dec 25, 2019, 4:19:28 AM12/25/19
to Review Board Community
Hi

Thanks for response, can I edit search.py to only get people except submitter? Will this have impact else where?

David Trowbridge

unread,
Dec 26, 2019, 4:03:53 PM12/26/19
to reviewboard
That's a generic API endpoint which is also used for the quick search in the search box at the top. Beyond that it's possible it's used elsewhere, but I don't know off the top of my head.

What exactly is the problem you're trying to solve? This sounds more like a culture and process issue than a technical one.

David

On Wed, Dec 25, 2019 at 2:19 AM santhosh K <keshw...@gmail.com> wrote:
Hi

Thanks for response, can I edit search.py to only get people except submitter? Will this have impact else where?

--
Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
---
You received this message because you are subscribed to the Google Groups "Review Board Community" group.
To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.

santhosh K

unread,
Dec 27, 2019, 1:08:45 AM12/27/19
to revie...@googlegroups.com
Wanted to avoid submitter as reviewer so that commit to svn doesn’t happen without proper reviewer input. 



z1p

unread,
Dec 27, 2019, 10:53:57 AM12/27/19
to Review Board Community
Hi Santosh,
I believe my first reaction to your question was inline with David`s last question. Which is why you feel you need thill enforces this restriction? I've been doing this work for many years and have found that in general engineers want to do the 'right' thing.

If you've found you have an engineer or two taking short cuts, then deal with them directly, don't take it out on your whole organization. If you aren't having issues and just want to put safeguards in place, don't! Engineers, like all employees, respond positively to trust, and negatively to distrust.

Paul Mansfield

unread,
Jan 2, 2020, 5:26:13 AM1/2/20
to Review Board Community
as a thought. is it possible with RB to disallow the person submitting the review from being able to Ship-It?
also, is it possible to make RB block the landing a review without two ship-it's from two different people?

this would help enforce good practises, many orgs including ours require two ship-its for merging to master.

Christian Hammond

unread,
Jan 3, 2020, 6:55:04 PM1/3/20
to revie...@googlegroups.com
Hi Paul,

The former is certainly worth considering (though when people are testing/evaluating, it's not uncommon for someone to review and Ship It their own changes).

The latter can be done by overriding approval logic using ReviewRequestApprovalHook in an extension, and by using `rbt land` (which provides some enforcement, but weaker as it's client-side) and/or a repository-side pre-commit hook that checks the approved flag for the review request referenced in commit messages (stronger enforcement).


Christian

--
Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
---
You received this message because you are subscribed to the Google Groups "Review Board Community" group.
To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.


--
Christian Hammond
President/CEO of Beanbag
Makers of Review Board
Reply all
Reply to author
Forward
0 new messages