[MPlayer-cvslog] CVS: main/libvo vo_macosx.m, 1.3, 1.4 vo_macosx.h, 1.1, 1.2

[Reimar Döffinger]

Hi,
On Mon, May 02, 2005 at 08:17:53PM -0400, Nicolas Plourde wrote:
> On 2-May-05, at 7:41 PM, Alexander Strasser wrote:
> Common sense should be the rule here. If the patch only change few lines
> of code and indentation is used to make things clearer, why not?

Yes, the point is making things clearer. And this makes the patch as it
appears in the cvslog _a lot_ less clear, since I can't immediately see
if you only added the if block or also changed the code.
That's why I also find the rule "it's okay if it is only a few lines"
nonsense.
And last but not least as I stated several times what is readable and
what is not is a matter of what you are used to. I have very little
problems with missing indentation whereas I find it very unreadable when
there are no spaces before and after =, == etc.
Since you are the maintainer in that case you get to decide about those
things, but please make it easy for people to review your changes, too.
As you should have a separate tree for development anyway it shouldn't
be too much work either, to create (with the right diff options) a patch
without all these cosmetics, apply that and then create one with only them.

Greetings,
Reimar Döffinger

P.S.: I see that vo_macosx.m uses OpenGL, too. It would be nice if yet
more code-duplication could be avoided, maybe have a look if some code
can be shared via gl_common.c (especially my OSD code in vo_gl.c is a
candidate for that IMHO)...