Re: [PATCH] Reformat sysdeps/x86/libc-start.c

[Carlos O'Donell <carlos at redhat dot com>]

On 10/30/2017 03:12 PM, Jonathan Nieder wrote:
> Hi,
> 
> Carlos O'Donell wrote:
> 
>> Should we extend consensus?
>>
>> https://sourceware.org/glibc/wiki/Consensus#Trivial_Bug-Fix_Changes
>> ~~~
>> Anyone can commit a change fixing obvious coding standards problems
>> in a recently committed patch. Post the patch and ChangeLog to
>> libc-alpha with a short message and then push the commit. 
>> ~~~
>> s/a recently/any/g
>>
>> We normally allow this kind of change for "recently committed"
>> patches, but shy away from it for older changes because of the impact
>> it might ave on established code.
>>
>> In this case I would have liked HJ to be able to just push the cleanup
>> without anyone *needing* to do a pre-commit review.
> 
> I would rather not --- getting LGTM is a pretty lightweight action,
> and in other projects when I thought I had a good cleanup, getting
> review pushed me in another direction that caused an even better
> result.

The point I made is not about any general changes, but about a specific
subset of changes, namely coding standard cleanups and trivial changes.

While perhaps there is a *better* result, the immediate incremental benefit
of HJ having checked in his patch, and moved on to another fix, is worth
codifying as acceptable community practice without needing immediate
review.

> I'd rather not see glibc switching to post-push review for most
> commits and pre-push review as an exception.  Instead, how can I help
> with making pre-push review work better?

We have very narrow criteria for post-push review. I do not think that
this will get abused by any serious developer in this community, and if
we see such abuse we can stop it.

We have post-push review for specific things mentioned here:
https://sourceware.org/glibc/wiki/Consensus

Even if we had more reviewer resources, having consensus only helps
accelerate the review for everyone (and you have to have commit access
in the first place).
 
> E.g. if obvious patches are stalling without review, can we figure out
> why and get that problem solved?  (E.g. do we need something like
> patchwork to keep track of patches needing review?)

How long is too long? 1h, 2h, 4h, 1day? What if you're working on the
weekend cleaning stuff up and nobody else is around to ack your cleanup?
We can have established consensus on some things, and having it just helps
make things move smoothly.

-- 
Cheers,
Carlos.