[MPlayer-cvslog] r24941 - trunk/mplayer.c

[Uoti Urpala]

On Mon, 2007-11-12 at 15:48 +0100, Dominik 'Rathann' Mierzejewski wrote:
> On Monday, 12 November 2007 at 14:36, Uoti Urpala wrote:
> > On Mon, 2007-11-12 at 00:07 +0100, Michael Niedermayer wrote:
> > > 2. cosmetic changes like reindent belong to seperate commits, consequently
> > > intermediate revisions will be badly indented
> > 
> > Depends on the size of the changes. Sometimes the logic changes can be
> > easier to see without reindentation but not always - at other times it's
> > easier to read them if the resulting code is correctly indented. Making
> > two commits is more work and higher overhead to read on -cvslog even in
> > the case where you find the isolated parts slightly easier to read.
> 
> The point (which I already made on IRC a while ago) is that you don't have
> to read through cosmetic-only commits. So the read overhead is actually
> a lot smaller.

As I said above leaving the reindentation out of the first commit does
not necessarily make it easier to read even in isolation. Also
completely skipping everything marked "cosmetic" on -cvslog means really
silly errors slip through easier.

> > Having twice as many commits in the revision history is a significant
> > drawback too.
> 
> How so? Besides, not every code change involves reindentation or other
> cosmetic changes. In fact, only a small minority of them do.
> Don't exaggerate it.

If you say that only a small minority of commits are affected it is an
argument for splitting/not splitting not mattering much either way. It
does not affect the relative merits of the alternatives; any benefit
from splitting would only affect the same "small minority" too.

> > > 3. the indention of changed code should be sane after you are finished
> > > with changing it in possibly several commits
> > > 
> > > i think the policy does say this already ...
> > 
> > Current text in svn-howto says NOT to do that if the reindent required
> > is over 5 lines (at least in the case where you add an if() over some
> > code).
> 
> Let's not delve into such detail. IMHO any cosmetic changes over one line
> should be separated and no exceptions.

You misunderstood what the subject was. It was not about splitting
changes. svn-howto.txt contains bad instructions telling not to fix the
indentation _at all_.

IMO one line is too small even as a guideline, and such formatting rules
generally should not be strict "no exceptions" rules.