Changes to our Git infrastructure

[Alexander Neundorf]

On Wednesday, January 07, 2015 08:03:06 Martin Gräßlin wrote:
> On Tuesday 06 January 2015 12:48:41 Jan Kundrát wrote:
> > On Tuesday, 6 January 2015 07:40:01 CEST, Ian Wadham wrote:
...
> > This has a risk of splitting the discussion about a patch into multiple
> > independent streams where people will have hard time keeping track of all
> > the results, IMHO.
> > 
> > > The polishing (fixing nitpicks, etc.) should come *after* the stone is
> > > cut.
> > 
> > That's a good suggestion.
> 
> I have to disagree here. I'm not able to do a good functional review if the
> change is full of unintended "nitpick" things. Each changed line triggers a
> "what is that change for?" and if it's just a change like
> - if (foo) {
> + if (foo)
> + {
> 
> then I have a hard time as a reviewer. For me as a reviewer it's important
> that the change is free of those "nitpick" things before I start looking at
> the functional things.

maybe there's a difference between nitpicking on useless changes, as above,  
(which increase the size of the patch and so make reviewing harder) and style 
issues in new code, where e.g. the indentation is slightly wrong, but that 
doesn't increase the patch size and so doesn't make reviewing harder.

Maybe there could be separate "design/approach review", a "coding style 
review" and a "code problems review", and a tool could even enforce an order 
between them. So nitpicking about wrong placement of curly braces in new code 
would come at the end, after the general idea has been accepted.
As I said above, keeping the size of a patch minimal is important.

Alex