[MPlayer-dev-eng] [PATCH] make cache2.c readable

[The Wanderer]

Uoti Urpala wrote:

> On Sun, 2006-09-10 at 15:26 -0400, The Wanderer wrote:
> 
>> Denis Vlasenko wrote:

>>> I didn't say "only whitespace" change. I said "No real code
>>> changes".
>> 
>> For one thing, how is changing a variable declaration to no longer
>> initialize the variable (when it will in fact be used before being
>> written to) not a real code change?
> 
> Because it doesn't change the declaration to "no longer initialize
> the variable". Static variables are initialized to 0 by default.

...live and learn...

Somehow I'd managed to make it this far in life without noticing that.
My bad.

>> For another, whitespace is not the only possible kind of cosmetic
>> issue, and all cosmetics are required to be in separate patches
>> from any other type of change. Even if you had added a "min_fill =
>> 0;" line elsewhere, to keep the same functionality, it would still
>> have been a cosmetic change and hence would have been unacceptable.
> 
> It was _supposed_ to be a cosmetic patch! It was said to be rejected
> because it was NOT a cosmetic change.

(Ergh. Another mixup. This time, I meant "excessive cosmetics" - i.e.
change without need for it, which is also generally not acceptable -
re-ordering statements whose order doesn't matter to the result, and so
forth. I should probably have omitted that last sentence entirely.)

>> I believe you had been told already that cosmetics must not be
>> mixed with any other kind of change. Why, then, make any
>> non-cosmetic change of any sort whatsoever in what is avowedly a
>> cosmetics patch?
> 
> Contradicts what you wrote above... and it was not a non-cosmetic
> change.

Both points already addressed, if somewhat poorly.

>>> But nevermind. I am not interested anymore in sending any sort of
>>> patch to this project.
>> 
>> I, for one, consider this somewhat unfortunate... the requirements
>> really aren't that onerous for the most part, just perhaps a bit
>> unexpected. You do seem (at least to my somewhat inexperienced eye)
>> the sort to be able to provide competent contributions, but you
>> don't seem willing to accept the reasons why these things are
>> required. (Somehow this paragraph doesn't quite say what I wanted
>> it to...)
> 
> I think his reaction was quite reasonable. And I think following the
> requirements strictly and completely literally like rejecting patches
> for containing one or two cosmetic changes IS onerous.

Your opinion on longstanding policy has been known for some time. While
I can see your point, and am unfond of utterly inflexible rules of
almost any sort, making exceptions to policy such as you seem to
advocate ("if there are only one or two cosmetic changes, accept it
anyway") almost negates the point of having the policy in the first
place. To my mind, the necessary flexibility is provided by the fact
that pure-cosmetics patches are considered acceptable, which provides a
way to make cosmetic changes when they are necessary.

-- 
       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

Secrecy is the beginning of tyranny.