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

[Alexander Strasser]

Diego Biurrun wrote:
> On Mon, May 02, 2005 at 08:31:28PM +0200, Reimar Döffinger wrote:
> > On Mon, May 02, 2005 at 08:06:02PM +0200, Nicolas Plourde CVS wrote:
> > > @@ -477,10 +482,13 @@
> > >  	//go fullscreen
> > >  	if(vo_fs)
> > >  	{
> > > -		//hide menubar and mouse if fullscreen on main display
> > > -		HideMenuBar();
> > > -		HideCursor();
> > > -		mouseHide = YES;
> > > +		if(!isRootwin)
> > > +		{
> > > +			//hide menubar and mouse if fullscreen on main display
> > > +			HideMenuBar();
> > > +			HideCursor();
> > > +			mouseHide = YES;
> > > +		}
> > 
> > Please avoid such cosmetics - if neccessary apply them seperately.
> 
> Depends on the definition of cosmetics..  He added an if().  I faintly
> remember flaming Alexander Strasser for sending a patch with similar
> cosmetics.  In the end I think it was decided that it was OK.

  Well, there was no real conclusion about it. But as these are only
a few lines (4), it is allowed as written in our CVS policy. Anyway
the change isn't purely cosmetic, it changed also the meaning as it
is not called always anymore (this is the idea of indenting it in
the first place).

  Tho i clearly understand that these changes aren't wanted for big
code parts, cause one might not initially see what is new and what is
old code from pure stats or from a quick look at the diff. And the other
problem being that a typo error could have sneaked in. The latter is
possible to check but it requires more work for the reviewer.

  So also i don't find it particurly good, i am for staying with the
current policy and allowing `cosmetic only' patches afterwards (at least
from the maintainers of the code in question ).
  My reason is that also this throws more work on the patch creator it
eases the position of the reviewer and i weight this a bit higher as
it imho (highly) helps code quality to have (more) reviewers.

  As a last thought the problem might be completely solvable by improving
technology, iirc Michael Niedermayer even had an idea as we discussed this
back then. Unfortunately no one (including me) had takin this idea further.

  Alex (beastd)