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

Uoti Urpala uoti.urpala at pp1.inet.fi
Mon Sep 11 02:43:29 CEST 2006


On Mon, 2006-09-11 at 01:10 +0200, Diego Biurrun wrote:
> On Mon, Sep 11, 2006 at 01:54:33AM +0300, Uoti Urpala wrote:
> > Yes, there are no reviewers for all patches. However I think the main
> > problem is that in many cases most people are not using/interested
> > in/familiar with the relevant code. The number of patches is not so high
> > that dealing with some cosmetic changes in each would cause significant
> > extra work.
> 
> I disagree completely.  I happen to be one of the (very few) active
> reviewers, so I'm not making this up.  Look at what Michael says and
> does, you'll see that he agrees.

I checked your commits during the last month which said they were
patches from others. There were 10 such commits. 5 were one-line
changes, one 2-line, one a deletion of 5 lines. Most of those were
related to the build system. For those the submission format or
cosmetics hardly matter. Of the remaining 3, 2 were larger changes to
the build system and one was the libswscale -fPIC support patch which
was mainly reviewed by Michael IIRC.

I think those numbers show that you do take care of the build system
patches, but not that some cosmetic changes per patch would be a problem
or that you would work much on patches related to code you don't
otherwise maintain yourself either.

> Dealing with stray cosmetics and similar issues that could easily be
> avoided does cause significant extra work.  Significant because it causes
> me frustration and wastes my time that I could better spend reviewing
> the next patch.  Significant because the bottleneck is at the reviewing
> side, not at the submitting side.

However rejecting patches because of some minor issues doesn't save time
on either side.

> I'll ask my question again: More reviewers are desperately needed.  Are
> you up to the task?

If you mean reviewing patches for features which I don't personally use
or care about, no.




More information about the MPlayer-dev-eng mailing list