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

[Uoti Urpala]

On Sun, 2006-09-10 at 15:26 -0400, The Wanderer wrote:
> Denis Vlasenko wrote:
> 
> > On Thursday 31 August 2006 20:23, Diego Biurrun wrote:
> > 
> >> On Sun, Aug 27, 2006 at 03:48:01PM +0200, Denis Vlasenko wrote:
> >> 
> >>> Patch makes indentation consistent and adds
> >>> some whitespace so that code is easier to read.
> >>> Some long lines wrapped.
> >>> 
> >>> No real code changes.
> >>> 
> >>> --- cache2.0/stream/cache2.c	2006-08-18 20:30:35.000000000 +0200
> >>> +++ cache2.1/stream/cache2.c	2006-08-21 20:15:43.000000000 +0200
> >>> @@ -35,333 +35,347 @@ int stream_seek_long(stream_t *s,off_t p
> >>> 
> >>> -static int min_fill=0;
> >>> +static int min_fill;
> >> 
> >> This is more than a whitespace change, patch rejected.  Whitespace
> >> changes must not be mixed with other changes.
> > 
> > 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.

> 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.

> 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.

> > 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.