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

[Diego Biurrun]

On Sun, Dec 11, 2011 at 09:08:36PM +0100, Alexander Strasser wrote:
> Diego Biurrun wrote:
> > On Sun, Dec 11, 2011 at 03:44:36PM +0100, ib wrote:
> > > 
> > > Log:
> > > Cosmetic: Adjust indent.
> > > 
> > > --- trunk/mplayer.c	Sun Dec 11 15:43:16 2011	(r34427)
> > > +++ trunk/mplayer.c	Sun Dec 11 15:44:36 2011	(r34428)
> > > @@ -3061,7 +3061,7 @@ play_next_file:
> > >                  if (cmd->id == MP_CMD_GUI)
> > >                      gui(GUI_RUN_MESSAGE, cmd->args[0].v.s);
> > >                  else
> > > -                gui(GUI_RUN_COMMAND, (void *)cmd->id);
> > > +                    gui(GUI_RUN_COMMAND, (void *)cmd->id);
> > >                  mp_cmd_free(cmd);
> > 
> > This is completely silly.  Fold such things into the original commit.
> 
>   Calling it "completely silly" sounds exaggerated to me.
> 
>   I don't have a problem with this commit and I do not think many developers
> have. Of course it might be convenient to not split this for the committer
> if he likes it that way.

This commit creates unnecessary clutter for no gain at all.  Now there is
one extra entry in the history (to skip over) and one extra mail that has
to be processed by everyone.  That's a lot of time wasted compared to the
gain, which is nil.  Add to that the time it took to split the change into
two commits in the first place.

We simply should not keep perpetuating this bad habit into the future,
but drop it instead.  No readability is gained by splitting off one-line
indentation changes into separate commits; it just wastes time.

>   As I understand it our coding policy says, to not include large indentations
> in a non-cosmetic commit. But it doesn't say you must include small cosmetics.

It also does not say "use your common sense" because that is kind of a given.

Diego