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

[Michael Niedermayer]

On Tue, Nov 13, 2007 at 05:29:49PM +0200, Uoti Urpala wrote:
> 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.

this surely is true for someone having infinite time but if the
time and will/interrest of a devleoper is limited then its better to
concentrate on the changes where the gain/cost ratio is best

also few are capable of reviewing functional changes, reviewing cosmetic
changes is very easy its just alot of work
if things are split properly the few who understand the code can concentrate
on reviewing functional changes while others who dont understand the code
could still review the cosmetic changes to check if its really just cosmetics

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-cvslog/attachments/20071113/470aff3b/attachment.pgp>