[Mplayer-cvslog] main/libvo vo_quartz.c,1.1,1.2

[Diego Biurrun]

Nicolas Plourde writes:
> Just to be clear, I know there like 2 cosmetic change in this
> patch but the previous file was like 3 month old and only
> applied 2 weeks ago by Alex. Its hard to keep my stuff up to  date
> at this rhythm and wait for every patch I submit to be committed.

We know it is inconvenient to maintain a complete file without CVS
access, that is why we gave it to you.

> Also the old file (1.1) did not have any support for fullscreen, ontop, 
> and OSD.
> These a major feature, but feel free to use the old and useless 1.1
> in place of 1.2 which is many way better.

Yes.  Without a doubt version 1.2 has more features and is better than
version 1.1.  But please read on for an explanation of why we have a
set of (strict) project rules that everybody must follow.

> 1.2 was the first file I committed myself and a fresh start for me
> since I USE TO have cvs access.

We know, and we know that beginners are prone to make changes, but as
a beginner you should also take special care to avoid mistakes.  The
problem was 

> I will now just submit patch to the list since, this way Alex will
> have chance to review and test every thing. And since he is more
> competent then me in carbon programming and mac related stuff maybe
> he can improve it too.

This is not about Alex or Arpi or me or others, it's a matter of
common sense and working as a team.  There have to be clear rules and
everybody has to abide by them.  It's also not really MPlayer
specific.  Yesterday we were talking about your commit in #mplayerdev,
how it was full of cosmetics and explaining to Reza Jelveh (timebomb)
why cosmetics are bad.  I'll copy and paste you some snippets from
that discussion.  jcdutton is James Courtier-Dutton from libdvdnav and
xine, dalias is Richard Felker and DonDiego is my humble self.


23:20 <+jcdutton> timebomb: NEVER mix code changes with cosmetic
changes.

23:23 <@DonDiego> timebomb: this is about working as a _team_. if you
work alone you can make all the cosmetic changes you want, but if you
make changes explicitly for the other devs to check, then a no
cosmetics policy is just plain common sense as jcdutton insightfully
remarked

23:27 <+jcdutton> timebomb: Take a simple example that you have one
line of code to change, but you add 100 lines of cosmetic changes in
the same patch. On a shared project, everyone wants to see what other
people are changing, and having to look through 100 lines when only a
single line has changes is just plain unacceptable to everyone.

23:27 <@dalias> cosmetic changes may not be mixed with patches which
change the code because it makes it VERY DIFFICULT to read the patch
and tell what's actually being changed in functionality so errors can
slip through (or intentional malicious code even!)

23:28 <+jcdutton> timebomb: It is perfectly ok to submit cosmetic
changes, but just don't mix then with code changes

23:29 <@dalias> and the mplayer rules (up til now anyway) have been
that you don't make cosmetic changes in other people's code at all if
we want to rethink that we can, but imo it's a good rule


So you see that we all share the same opinion and even the xine people
agree.  It really is a common sense thing.  In a project of this scale
it is not feasible to work in any other way.

> Sorry for wasting your time trying to improve this nice software.

Nicolas, please do not take this as a personal insult.  It is not
meant to be one.  I hope to have explained above why our rules are not
meant to make your life difficult but reasonable and necessary.

Another thing is that you committed several big independent changes
all at once.  It is best to separate changes as much as possible.
This makes later debugging much easier when you can use your version
control system to easily rip out parts of the code and see what is
causing the trouble.  You have sent a few patches for OSD, fullscreen,
ontop and maybe other things, I don't remember all the details.  It
would be best if you could commit those separately, you should have
those patches lying around.

I'm sure you can get back your CVS access, it's just that you have to
understand the rules and follow them, OK?

I'll try writing up a more explicit and extensive rules document in
the future.  Hints where our cvs-howto.txt is lacking would be
welcome.

Please keep up the good work.
Regards

Diego