[MPlayer-cvslog] CVS: main/libmpdemux demux_ogg.c,1.69,1.70

[Reimar Döffinger]

Hi,
On Wed, May 04, 2005 at 06:04:22PM -0500, Joey Parrish wrote:
> On Wed, May 04, 2005 at 10:57:32AM +0200, Dominik 'Rathann' Mierzejewski wrote:
> > > The newline doesn't matter.  It is effectively the same as:
> > > demuxer->priv = ogg_d = (ogg_demuxer_t*)calloc(1,sizeof(ogg_demuxer_t));
> > > Both demuxer->priv and ogg_d will have the same value.
> 
> > You are correct. However, this would've been much more readable without
> > the newline.
> 
> This is why I think the cosmetic policy is too strict.  People are doing
> this kind of thing to avoid "cosmetic" changes.  Readability, especially
> as it pertains to the actual code you're altering, should come first.
> This is a good example of that.

First, yes, readability should come first - but of the code _and_ the
patch. Which is why I am for the two-step approach. Patch readability is
very important, because most developers will have a look at them, while
most won't have a look at the real code unless they maintain it or try
to find a bug in it - which is another point why I find it more
acceptable to tell people: "well, if the missing indentation annoys you,
use indent on the file" than saying "if you don't like the cosmetics do
cvs diff -ub".
Also this IMO wasn't a case of the anti-cosmetics rule, I just found it
made no difference to the readability of the resulting code and made the
patch a tiny bit easier to understand - well, maybe I was wrong, but
after all I posted the patch to -dev-eng first for a reason...

Greetings,
Reimar Döffinger