[Mplayer-cvslog] CVS: main/libmpcodecs vd_xvid4.c,1.2,1.3

[Reimar Döffinger]

Hi,

> Another thing that bugs me, is that nobody seem to care to explain me
> why cosmetics change is a no-no, especially for a piece of code that
> gets rarely updated, and on which no many people work. I do understand
> that this is different for mplayer.c.

Not-so hypothetical case: you have a file that doesn't work but you know 
it worked somewhen. So you go through the revision to find the patch 
that broke it.
Now try to guess how hard it is to find the thing that caused it to 
break when the patch consists of hundreds of changed lines but most of 
them are only cosmetics. You would have to check each of those whether 
they changed something or not.
With a cosmestics-free patch you'd only have to have a look at the few 
that really changed the code.
This would be fixed by committing code and cosmetics separately, but 
then you still have the problem when the code was not broken by one 
patch but a combination of two, with cosmetics committed in betwee those 
two. Same problem again.
And something that at least for me counts as well is that it's MPlayer's 
style. This is enough for me that it needs a really good reason to 
change it, and not a really good reason to stay as it is ;-)
Please also take into account when judging other people's responses to 
this, that the cosmetics rule was discussed over and over and people get 
really fed up with it. At least I'm getting and I joined MPlayer only a 
(relatively?) short time ago.

> I do agree that a cosmetic change such as a variable name change isn't
> maybe worth it, but I also think it's really too bad that it's the only
> thing that caught your attention, when I'd really have prefer not to
> feel alone with the issue of moving to a 1.1.x-compatible front-end.

I'm not qualified to comment on anything beyond that. And some others 
might be especially unwilling to check a patch that does not at least 
"look clean".

> First, because I trust Edouard judgment regarding producing good code,
> as I really see no good reason why he'd screw the front-end in purpose.

Noone thinks any of you would screw it on purpose, but producing 
error-free code is (almost?) an impossibility.

> Right now, I'd really prefer to commit the second half of my patches
> which I posted some days ago, that feature both real code and
> documentation, and move on.

I can sure understand that. Waiting forever for a patch to get applied 
or even checked is frustrating.
Nevertheless, I prefer not to take sides, the truth being that I really 
don't care about XvID support - the reason being simply that don't even 
have it installed and I'm occupied with fixing other things. Also I 
actually hardly use MPlayer at all...

>>Guillaume is not subscribed to -cvslog.  Guillaume, this is something
>>you have to do.  It's also written in cvs-howto.txt.  If you commit
>>things you have to be available to fix your mistakes.
> 
> Now I'm subscribed to -cvslog. I wasn't previously as I thought it was a
> read-only list that featured only messages from the cvs daemon.
> I'm maybe playing with words, but so far, I wouldn't call what I've done
> a "mistake", more a policy violation, even though I regret people take
> it that way.

Well, the cvs-howto.txt exists for being followed, especially by anyone 
new. And you should also read it closely to please Diego, he took quite 
some time to work out many of the details in it ;-)

> Anyway, thanks Reimar and Diego to plead in my favor, I really
> appreciate it. I really want to help, and provide good help, but I also
> what to understand.

My reply was in defense, not exactly in favor. I don't want you to be 
scared away ;-)
I am one of those who are opposed to cosmetic changes except in some 
very specific cases. My goal is: make your patch as small and 
understandable as possible. So should you ever commit cosmetics to code 
maintained or written by me expect a similar response ;-)

Greetings,
Reimar Döffinger