[MPlayer-dev-eng] [PATCH] libvo: GGI driver update

[Reimar Döffinger]

Hi,
On Tue, Sep 06, 2005 at 03:24:36PM +0200, Christoph Egger wrote:
> > So the switch and case statements are supposed to be on the same level?
> 
> Yes. What you see is the switch to K&R C-style. Linux, *BSD
> and many other projects use it. It's a quasi-standard.

Last I heard linux is in the transition, not yet finished, but I'm not
up to date. I just wanted to make sure this wasn't accidential.

> What is the mplayer's _general_ coding style?

Whatever the surrounding code uses, though spaces are prefered over tabs
since the usually work out better (well, at least that was my impression
of the general opinion).
And there is a no-cosmetics rule, though I don't flame maintainers for
it (anymore?).

> > And there still are space used e.g. here (line 398):
> >  #if 1
> > -       (IMGFMT_IS_RGB(mpi->imgfmt) &&
> > +           (IMGFMT_IS_RGB(mpi->imgfmt) &&
> >             (IMGFMT_RGB_DEPTH(mpi->imgfmt) != vo_dbpp)) ||
> > -       (IMGFMT_IS_BGR(mpi->imgfmt) &&
> > +           (IMGFMT_IS_BGR(mpi->imgfmt) &&
> >             (IMGFMT_BGR_DEPTH(mpi->imgfmt) != vo_dbpp)) ||
> > 
> > I can't believe I'm reviewing a cosmetics patch, and to put it bluntly,
> 
> As I told above, this is K&R coding style.

Just though the point was to get completely rid of the tabs + spaces
mixture.

> > I don't know if you'll actually find someone applying this
> 
> You give me the impression the quality/readability of the patch counts
> rather the result after applying the patch...

Yes, it is a patches that change, improve and break things. And it is
reviewing patches that slows down development most.

> > (unless you get CVS access yourself).
> 
> I'm told be the maintainer for the ggi driver (However, I'm not aware
> of any account information). The code in CVS (up to rev 1.37) is a mix

I don't remember how that came about, I just looked in
DOCS/tech/MAINTAINERS. Usually maintainers get CVS access, but also I
think you usually don't become a maintainer that fast.

> As a maintainer I should have the right to
> say 'Stop. Please don't make it worse...'

That certainly *g*. Here the point is more if you have the right to
"fix" it.
Here you need somebody to agree with it and take the flames and drink
the cola in case it breaks something.
Ivan, can you comment on it? I think you have more "understanding" for
this kind of patch. Or somebody else who is interested in keeping the
only person improving vo_ggi happy *g*

Greetings,
Reimar Döffinger

P.S. Sorry that this is such a pain, but after so many (some valid)
complaints about the no cosmetics rule, and IMHO as many good points for
it the situation is rather unclear, at least to me...
I personally would agree on a one-time cosmetics change upon a
maintainer change, although one of this size will break cvs diffs
between versions before and after. I am decidedly undecided.