Re: Commits without reviews

[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 
wrote:

> 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.
>
> On Tue, Dec 29, 2015 at 8:51 PM, Dave Lester  wrote:
> > 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, 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
> >>>
> >>> On Tue, Dec 29, 2015 at 8:00 PM, John Sirois 
> wrote:
>  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 projects where the right answer should
>  generally be one either roll back or fork/patch/custom private release
> >>> for
>  the emergency.
>  My experience is biased by the 1 spat of TBR I personally did as
> >>> encouraged
>  by the core pants committers on the pants project.  This was a series
> of
>  ~20 TBR reviews of experimental code in an exp/ dir unused by the
> >>> mainline
>  code, but committed to master.  The hope was that these reviews would
> be
>  looked at and they have not been ~3 months down the road.
> 
>  On Tue, Dec 29, 2015 at 8:38 PM, Jake Farrell 
> >>> wrote:
> 
> > +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 below for
> some
> >> rationale, but i would like to propose that we allow committers to
> > exercise
> >> judgement on skipping code reviews for changes unrelated to build or
> >>> test
> >> of the main project (e.g. scheduler, executor, client, packaging).
> >>> What
> > do
> >> you all think?
> >>
> >> As an example, i think the code review process is too much overhead
> >>> for
> >> commits like the ones below.  With these commits i was playing
> > whack-a-mole
> >> to get alignment between markdown rendering on
> >>> github.com/apache/aurora
> >> and
> >> aurora.apache.org.  Skipping code review allowed me to fix things
> in
> >>> a
> >> much
> >> shorter timeframe.
> >>
> >> commit 0d9fe18
> >> Author: Bill Farner 
> >> Date:   Wed Dec 23 08:31:27 2015 -0800
> >>
> >>Fix string interpolation for release email.
> >>
> >> commit df5200b
> >> Author: Bill Farner 
> >> Date:   Mon Dec 21 14:19:48 2015 -0800
> >>
> >>Fix formatting and work around anchor link issues in
> >>> installing.md
> >>
> >> commit 21c605e
> >> Author: Bill Farner 
> >> Date:   Mon Dec 21 14:11:10 2015 -0800
> >>
> >>Fix anchor links in installing.md.
> >>
> >> commit 9326fa6
> >> Author: Bill Farner 
> >> Date:   Mon Dec 21 12:21:37 2015 -0800
> >>
> >>Link to install guide from docs/README.md
> >>
> >> commit f8e59a4
> >> Author: Bill Farner 
> >> Date:   Mon Dec 21 12:12:56 2015 -0800
> >>
> >>Fix formatting issues in installing doc.
> >>
> >
> 
> 
> 
>  --
>  John Sirois
>  303-512-3301
> >>>
> >
>

Re: Commits without reviews

[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
>
> On Tue, Dec 29, 2015 at 8:00 PM, John Sirois  wrote:
> > 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 projects where the right answer should
> > generally be one either roll back or fork/patch/custom private release
> for
> > the emergency.
> > My experience is biased by the 1 spat of TBR I personally did as
> encouraged
> > by the core pants committers on the pants project.  This was a series of
> > ~20 TBR reviews of experimental code in an exp/ dir unused by the
> mainline
> > code, but committed to master.  The hope was that these reviews would be
> > looked at and they have not been ~3 months down the road.
> >
> > On Tue, Dec 29, 2015 at 8:38 PM, Jake Farrell 
> wrote:
> >
> >> +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 below for some
> >> > rationale, but i would like to propose that we allow committers to
> >> exercise
> >> > judgement on skipping code reviews for changes unrelated to build or
> test
> >> > of the main project (e.g. scheduler, executor, client, packaging).
> What
> >> do
> >> > you all think?
> >> >
> >> > As an example, i think the code review process is too much overhead
> for
> >> > commits like the ones below.  With these commits i was playing
> >> whack-a-mole
> >> > to get alignment between markdown rendering on
> github.com/apache/aurora
> >> > and
> >> > aurora.apache.org.  Skipping code review allowed me to fix things in
> a
> >> > much
> >> > shorter timeframe.
> >> >
> >> > commit 0d9fe18
> >> > Author: Bill Farner 
> >> > Date:   Wed Dec 23 08:31:27 2015 -0800
> >> >
> >> > Fix string interpolation for release email.
> >> >
> >> > commit df5200b
> >> > Author: Bill Farner 
> >> > Date:   Mon Dec 21 14:19:48 2015 -0800
> >> >
> >> > Fix formatting and work around anchor link issues in
> installing.md
> >> >
> >> > commit 21c605e
> >> > Author: Bill Farner 
> >> > Date:   Mon Dec 21 14:11:10 2015 -0800
> >> >
> >> > Fix anchor links in installing.md.
> >> >
> >> > commit 9326fa6
> >> > Author: Bill Farner 
> >> > Date:   Mon Dec 21 12:21:37 2015 -0800
> >> >
> >> > Link to install guide from docs/README.md
> >> >
> >> > commit f8e59a4
> >> > Author: Bill Farner 
> >> > Date:   Mon Dec 21 12:12:56 2015 -0800
> >> >
> >> > Fix formatting issues in installing doc.
> >> >
> >>
> >
> >
> >
> > --
> > John Sirois
> > 303-512-3301
>

Re: Commits without reviews

[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 below for some
> rationale, but i would like to propose that we allow committers to exercise
> judgement on skipping code reviews for changes unrelated to build or test
> of the main project (e.g. scheduler, executor, client, packaging).  What do
> you all think?
>
> As an example, i think the code review process is too much overhead for
> commits like the ones below.  With these commits i was playing whack-a-mole
> to get alignment between markdown rendering on github.com/apache/aurora
> and
> aurora.apache.org.  Skipping code review allowed me to fix things in a
> much
> shorter timeframe.
>
> commit 0d9fe18
> Author: Bill Farner 
> Date:   Wed Dec 23 08:31:27 2015 -0800
>
> Fix string interpolation for release email.
>
> commit df5200b
> Author: Bill Farner 
> Date:   Mon Dec 21 14:19:48 2015 -0800
>
> Fix formatting and work around anchor link issues in installing.md
>
> commit 21c605e
> Author: Bill Farner 
> Date:   Mon Dec 21 14:11:10 2015 -0800
>
> Fix anchor links in installing.md.
>
> commit 9326fa6
> Author: Bill Farner 
> Date:   Mon Dec 21 12:21:37 2015 -0800
>
> Link to install guide from docs/README.md
>
> commit f8e59a4
> Author: Bill Farner 
> Date:   Mon Dec 21 12:12:56 2015 -0800
>
> Fix formatting issues in installing doc.
>

Re: Commits without reviews

[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.

On Tue, Dec 29, 2015 at 8:51 PM, Dave Lester  wrote:
> 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, 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
>>>
>>> On Tue, Dec 29, 2015 at 8:00 PM, John Sirois  wrote:
 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 projects where the right answer should
 generally be one either roll back or fork/patch/custom private release
>>> for
 the emergency.
 My experience is biased by the 1 spat of TBR I personally did as
>>> encouraged
 by the core pants committers on the pants project.  This was a series of
 ~20 TBR reviews of experimental code in an exp/ dir unused by the
>>> mainline
 code, but committed to master.  The hope was that these reviews would be
 looked at and they have not been ~3 months down the road.

 On Tue, Dec 29, 2015 at 8:38 PM, Jake Farrell 
>>> wrote:

> +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 below for some
>> rationale, but i would like to propose that we allow committers to
> exercise
>> judgement on skipping code reviews for changes unrelated to build or
>>> test
>> of the main project (e.g. scheduler, executor, client, packaging).
>>> What
> do
>> you all think?
>>
>> As an example, i think the code review process is too much overhead
>>> for
>> commits like the ones below.  With these commits i was playing
> whack-a-mole
>> to get alignment between markdown rendering on
>>> github.com/apache/aurora
>> and
>> aurora.apache.org.  Skipping code review allowed me to fix things in
>>> a
>> much
>> shorter timeframe.
>>
>> commit 0d9fe18
>> Author: Bill Farner 
>> Date:   Wed Dec 23 08:31:27 2015 -0800
>>
>>Fix string interpolation for release email.
>>
>> commit df5200b
>> Author: Bill Farner 
>> Date:   Mon Dec 21 14:19:48 2015 -0800
>>
>>Fix formatting and work around anchor link issues in
>>> installing.md
>>
>> commit 21c605e
>> Author: Bill Farner 
>> Date:   Mon Dec 21 14:11:10 2015 -0800
>>
>>Fix anchor links in installing.md.
>>
>> commit 9326fa6
>> Author: Bill Farner 
>> Date:   Mon Dec 21 12:21:37 2015 -0800
>>
>>Link to install guide from docs/README.md
>>
>> commit f8e59a4
>> Author: Bill Farner 
>> Date:   Mon Dec 21 12:12:56 2015 -0800
>>
>>Fix formatting issues in installing doc.
>>
>



 --
 John Sirois
 303-512-3301
>>>
>

Re: Commits without reviews

[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 projects where the right answer should
generally be one either roll back or fork/patch/custom private release for
the emergency.
My experience is biased by the 1 spat of TBR I personally did as encouraged
by the core pants committers on the pants project.  This was a series of
~20 TBR reviews of experimental code in an exp/ dir unused by the mainline
code, but committed to master.  The hope was that these reviews would be
looked at and they have not been ~3 months down the road.

On Tue, Dec 29, 2015 at 8:38 PM, Jake Farrell  wrote:

> +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 below for some
> > rationale, but i would like to propose that we allow committers to
> exercise
> > judgement on skipping code reviews for changes unrelated to build or test
> > of the main project (e.g. scheduler, executor, client, packaging).  What
> do
> > you all think?
> >
> > As an example, i think the code review process is too much overhead for
> > commits like the ones below.  With these commits i was playing
> whack-a-mole
> > to get alignment between markdown rendering on github.com/apache/aurora
> > and
> > aurora.apache.org.  Skipping code review allowed me to fix things in a
> > much
> > shorter timeframe.
> >
> > commit 0d9fe18
> > Author: Bill Farner 
> > Date:   Wed Dec 23 08:31:27 2015 -0800
> >
> > Fix string interpolation for release email.
> >
> > commit df5200b
> > Author: Bill Farner 
> > Date:   Mon Dec 21 14:19:48 2015 -0800
> >
> > Fix formatting and work around anchor link issues in installing.md
> >
> > commit 21c605e
> > Author: Bill Farner 
> > Date:   Mon Dec 21 14:11:10 2015 -0800
> >
> > Fix anchor links in installing.md.
> >
> > commit 9326fa6
> > Author: Bill Farner 
> > Date:   Mon Dec 21 12:21:37 2015 -0800
> >
> > Link to install guide from docs/README.md
> >
> > commit f8e59a4
> > Author: Bill Farner 
> > Date:   Mon Dec 21 12:12:56 2015 -0800
> >
> > Fix formatting issues in installing doc.
> >
>



-- 
John Sirois
303-512-3301

Re: Commits without reviews

[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, 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
>> 
>> On Tue, Dec 29, 2015 at 8:00 PM, John Sirois  wrote:
>>> 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 projects where the right answer should
>>> generally be one either roll back or fork/patch/custom private release
>> for
>>> the emergency.
>>> My experience is biased by the 1 spat of TBR I personally did as
>> encouraged
>>> by the core pants committers on the pants project.  This was a series of
>>> ~20 TBR reviews of experimental code in an exp/ dir unused by the
>> mainline
>>> code, but committed to master.  The hope was that these reviews would be
>>> looked at and they have not been ~3 months down the road.
>>> 
>>> On Tue, Dec 29, 2015 at 8:38 PM, Jake Farrell 
>> wrote:
>>> 
 +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 below for some
> rationale, but i would like to propose that we allow committers to
 exercise
> judgement on skipping code reviews for changes unrelated to build or
>> test
> of the main project (e.g. scheduler, executor, client, packaging).
>> What
 do
> you all think?
> 
> As an example, i think the code review process is too much overhead
>> for
> commits like the ones below.  With these commits i was playing
 whack-a-mole
> to get alignment between markdown rendering on
>> github.com/apache/aurora
> and
> aurora.apache.org.  Skipping code review allowed me to fix things in
>> a
> much
> shorter timeframe.
> 
> commit 0d9fe18
> Author: Bill Farner 
> Date:   Wed Dec 23 08:31:27 2015 -0800
> 
>Fix string interpolation for release email.
> 
> commit df5200b
> Author: Bill Farner 
> Date:   Mon Dec 21 14:19:48 2015 -0800
> 
>Fix formatting and work around anchor link issues in
>> installing.md
> 
> commit 21c605e
> Author: Bill Farner 
> Date:   Mon Dec 21 14:11:10 2015 -0800
> 
>Fix anchor links in installing.md.
> 
> commit 9326fa6
> Author: Bill Farner 
> Date:   Mon Dec 21 12:21:37 2015 -0800
> 
>Link to install guide from docs/README.md
> 
> commit f8e59a4
> Author: Bill Farner 
> Date:   Mon Dec 21 12:12:56 2015 -0800
> 
>Fix formatting issues in installing doc.
> 
 
>>> 
>>> 
>>> 
>>> --
>>> John Sirois
>>> 303-512-3301
>> 

Re: Commits without reviews

[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  wrote:

> +1
>
> Flying by. Mesos allows small commits without reviews and it works well in
> practice.
>
> - Jie
>
> On Wed, Dec 23, 2015 at 1: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 below for some
> > rationale, but i would like to propose that we allow committers to
> exercise
> > judgement on skipping code reviews for changes unrelated to build or test
> > of the main project (e.g. scheduler, executor, client, packaging).  What
> do
> > you all think?
> >
> > As an example, i think the code review process is too much overhead for
> > commits like the ones below.  With these commits i was playing
> whack-a-mole
> > to get alignment between markdown rendering on github.com/apache/aurora
> > and
> > aurora.apache.org.  Skipping code review allowed me to fix things in a
> > much
> > shorter timeframe.
> >
> > commit 0d9fe18
> > Author: Bill Farner 
> > Date:   Wed Dec 23 08:31:27 2015 -0800
> >
> > Fix string interpolation for release email.
> >
> > commit df5200b
> > Author: Bill Farner 
> > Date:   Mon Dec 21 14:19:48 2015 -0800
> >
> > Fix formatting and work around anchor link issues in installing.md
> >
> > commit 21c605e
> > Author: Bill Farner 
> > Date:   Mon Dec 21 14:11:10 2015 -0800
> >
> > Fix anchor links in installing.md.
> >
> > commit 9326fa6
> > Author: Bill Farner 
> > Date:   Mon Dec 21 12:21:37 2015 -0800
> >
> > Link to install guide from docs/README.md
> >
> > commit f8e59a4
> > Author: Bill Farner 
> > Date:   Mon Dec 21 12:12:56 2015 -0800
> >
> > Fix formatting issues in installing doc.
> >
>

Re: Commits without reviews

[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:
>
> > 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
> > be followed up in a subsequent commit if needed).
> >
> > On Wed, Dec 23, 2015 at 3: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 below for some
> > > rationale, but i would like to propose that we allow committers to
> > exercise
> > > judgement on skipping code reviews for changes unrelated to build or
> test
> > > of the main project (e.g. scheduler, executor, client, packaging).
> What
> > do
> > > you all think?
> > >
> > > As an example, i think the code review process is too much overhead for
> > > commits like the ones below.  With these commits i was playing
> > whack-a-mole
> > > to get alignment between markdown rendering on
> github.com/apache/aurora
> > > and
> > > aurora.apache.org.  Skipping code review allowed me to fix things in a
> > > much
> > > shorter timeframe.
> > >
> > > commit 0d9fe18
> > > Author: Bill Farner >
> > > Date:   Wed Dec 23 08:31:27 2015 -0800
> > >
> > > Fix string interpolation for release email.
> > >
> > > commit df5200b
> > > Author: Bill Farner >
> > > Date:   Mon Dec 21 14:19:48 2015 -0800
> > >
> > > Fix formatting and work around anchor link issues in installing.md
> > >
> > > commit 21c605e
> > > Author: Bill Farner >
> > > Date:   Mon Dec 21 14:11:10 2015 -0800
> > >
> > > Fix anchor links in installing.md.
> > >
> > > commit 9326fa6
> > > Author: Bill Farner >
> > > Date:   Mon Dec 21 12:21:37 2015 -0800
> > >
> > > Link to install guide from docs/README.md
> > >
> > > commit f8e59a4
> > > Author: Bill Farner >
> > > Date:   Mon Dec 21 12:12:56 2015 -0800
> > >
> > > Fix formatting issues in installing doc.
> > >
> >
>

Re: Commits without reviews

[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  wrote:

> 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:
> >
> > > 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
> > > be followed up in a subsequent commit if needed).
> > >
> > > On Wed, Dec 23, 2015 at 3: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 below for some
> > > > rationale, but i would like to propose that we allow committers to
> > > exercise
> > > > judgement on skipping code reviews for changes unrelated to build or
> > test
> > > > of the main project (e.g. scheduler, executor, client, packaging).
> > What
> > > do
> > > > you all think?
> > > >
> > > > As an example, i think the code review process is too much overhead
> for
> > > > commits like the ones below.  With these commits i was playing
> > > whack-a-mole
> > > > to get alignment between markdown rendering on
> > github.com/apache/aurora
> > > > and
> > > > aurora.apache.org.  Skipping code review allowed me to fix things
> in a
> > > > much
> > > > shorter timeframe.
> > > >
> > > > commit 0d9fe18
> > > > Author: Bill Farner >
> > > > Date:   Wed Dec 23 08:31:27 2015 -0800
> > > >
> > > > Fix string interpolation for release email.
> > > >
> > > > commit df5200b
> > > > Author: Bill Farner >
> > > > Date:   Mon Dec 21 14:19:48 2015 -0800
> > > >
> > > > Fix formatting and work around anchor link issues in
> installing.md
> > > >
> > > > commit 21c605e
> > > > Author: Bill Farner >
> > > > Date:   Mon Dec 21 14:11:10 2015 -0800
> > > >
> > > > Fix anchor links in installing.md.
> > > >
> > > > commit 9326fa6
> > > > Author: Bill Farner >
> > > > Date:   Mon Dec 21 12:21:37 2015 -0800
> > > >
> > > > Link to install guide from docs/README.md
> > > >
> > > > commit f8e59a4
> > > > Author: Bill Farner >
> > > > Date:   Mon Dec 21 12:12:56 2015 -0800
> > > >
> > > > Fix formatting issues in installing doc.
> > > >
> > >
> >
>

Re: Commits without reviews

[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
be followed up in a subsequent commit if needed).

On Wed, Dec 23, 2015 at 3: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 below for some
> rationale, but i would like to propose that we allow committers to exercise
> judgement on skipping code reviews for changes unrelated to build or test
> of the main project (e.g. scheduler, executor, client, packaging).  What do
> you all think?
>
> As an example, i think the code review process is too much overhead for
> commits like the ones below.  With these commits i was playing whack-a-mole
> to get alignment between markdown rendering on github.com/apache/aurora
> and
> aurora.apache.org.  Skipping code review allowed me to fix things in a
> much
> shorter timeframe.
>
> commit 0d9fe18
> Author: Bill Farner 
> Date:   Wed Dec 23 08:31:27 2015 -0800
>
> Fix string interpolation for release email.
>
> commit df5200b
> Author: Bill Farner 
> Date:   Mon Dec 21 14:19:48 2015 -0800
>
> Fix formatting and work around anchor link issues in installing.md
>
> commit 21c605e
> Author: Bill Farner 
> Date:   Mon Dec 21 14:11:10 2015 -0800
>
> Fix anchor links in installing.md.
>
> commit 9326fa6
> Author: Bill Farner 
> Date:   Mon Dec 21 12:21:37 2015 -0800
>
> Link to install guide from docs/README.md
>
> commit f8e59a4
> Author: Bill Farner 
> Date:   Mon Dec 21 12:12:56 2015 -0800
>
> Fix formatting issues in installing doc.
>