Re: Commits without reviews

2016-01-06 Thread Bill Farner
To wrap up this discussion: - we WILL NOT allow 'TBR' reviews of code - we WILL allow trivial commits to non-code parts of the repo without a review If anyone disagrees, or feels this does not capture the discussion, please speak up! On Tue, Dec 29, 2015 at 10:07 PM, Maxim Khutornenko

Re: Commits without reviews

2015-12-29 Thread Bill Farner
Sorry for the jargon - "to be reviewed". It's a commit that is reviewed offline, with the expectation that the committer will address any comments in a follow-up patch. On Tue, Dec 29, 2015 at 8:35 PM, Henry Saputra wrote: > I am sorry, but what is TBR? > > - Henry > >

Re: Commits without reviews

2015-12-29 Thread Jake Farrell
+1 -Jake On Wed, Dec 23, 2015 at 4:48 PM, Bill Farner wrote: > All, > > Over the past few days, i have made several commits to the repository > without code review. Our convention has historically been to perform a > code review for any change, however small. Please see

Re: Commits without reviews

2015-12-29 Thread Maxim Khutornenko
The original proposal Bill made was "...for changes unrelated to build or test of the main project (e.g. scheduler, executor, client, packaging)..." +1 on either skipping the RB or TBR for any changes falling into above category. -1 for sidestepping the official review process for anything else.

Re: Commits without reviews

2015-12-29 Thread John Sirois
I'm +1 to skipping reviews for those portions of the codebase that are hard to test except via trail and error. I'm -0 to using TBR in an OSS project. In my mind TBR is for emregencies of which there should be none in an OSS infra project; these should only be in the leaves that use the OSS

Re: Commits without reviews

2015-12-29 Thread Dave Lester
I’m -1 to TBR in most cases. Exceptions may be where there is clear community consensus, and a design document that has been discussed. > On Dec 29, 2015, at 11:48 PM, Bill Farner wrote: > > Sorry for the jargon - "to be reviewed". It's a commit that is reviewed > offline,

Re: Commits without reviews

2015-12-24 Thread Bill Farner
Thinking more about what Joshua said above, i think we could also have a valuable discussion about making more frequent use of "to be reviewed" (a.k.a. tbr) reviews that we commit before receiving review feedback. Anybody have thoughts on that? On Thu, Dec 24, 2015 at 10:25 AM, Jie Yu

Re: Commits without reviews

2015-12-24 Thread Joshua Cohen
It could be, I just think it's easier to comment on a reviewboard than it is a commits@ email. On Thu, Dec 24, 2015 at 10:29 AM, Bill Farner wrote: > Can that be handled by subscribing to commits@? > > On Thursday, December 24, 2015, Joshua Cohen wrote: >

Re: Commits without reviews

2015-12-24 Thread Bill Farner
This is where i look towards allowing committers to exercise judgement. IMHO for commits like the ones above, a review is extra noise for everyone. I suppose my position is that i favor simplifying commits over simplifying comments. On Thu, Dec 24, 2015 at 9:12 AM, Joshua Cohen

Re: Commits without reviews

2015-12-24 Thread Joshua Cohen
I'm generally ok with this. Just curious: what do you think about maybe posting a review and then committing it right away in these cases though? A bit noisy on the reviews@ list, but at least it'd give people a chance to peruse/comment as they see fit (with the assumption that any comments would