[MPlayer-dev-eng] cvs-howto.txt

[Roberto Togni]

On 2004.08.30 14:33, Diego Biurrun wrote:
> Attila Kinali writes:
[...]
> I've included the main block of cvs-howto.txt from my local tree with
> all the changes suggested above included.
> 
> Please comment.
> 
> Diego
>
[...]
> 4. Adding new files/dirs:
> 
>     cvs add filename/dirname
add also "cvs commit filename" here

> 
> 5. Removing files:
> 
>     rm filename
>     cvs remove filename
>     cvs commit filename
> 
> 6. Checking changes:
> 
>     cvs -z3 diff -u filename(s)
>
maybe cvs update before doing it? i usually do it, if version number  
does not match with current cvs commit is aborted.
you can also use cvs status to check for it iirc

>   Doublecheck your changes before committing to avoid trouble later
> on.
>   This way you will see if your patch has debug stuff or indentation
>   changes and you can fix it before committing and triggering flames.
> 
> 7. Checking changelog:
> 
>     cvs -z3 log filename(s)
>     cvs -z3 annotate filename(s)
Please mention also cvsweb, it's a lot easier to track changes with  
that than with log

[...]
> 9. Reverting broken commits
> 
>   In case you committed something really broken and wish to undo it
> completely,
>   you can use the 'cvs admin -o' command, which removes entries from
> the
>   revision history of a file. It does not undo the changes related to
> that
>   revision, but for the corner case that you remove the last revision
> (and only
>   then!) this amounts to reverting a commit.
> 
>   Assuming that 1.123 is the last revision
> 
>     cvs -z3 admin -o1.123 filename
> 
>   will remove revision 1.123, thus reverting the file back to  
> revision
> 1.122.
> 
>   ONLY use this command to delete the LAST revision of a file.
> Removing other
>   revisions will NOT undo the changes connected to that revision, so
> the last
>   revision will remain unmodified, only revision history will be lost
> and
>   holes left in the revision numbering.
I would add some clarification here (or maybe i  the policy section):
Revert a change if and only if:
- you made something stupid, like committing the wrong version of a  
patch, some broken code or cosmetics, and you're going to recommit the  
right version immediately
- you committed something that was not intended to be committed (like  
your private debug version of another file not involved in your  
changes)

Never revert:
- patches committed long time before the reversal (remove them in the  
traditional way, giving a reason for it)
- fix that didn't work as expected, or worked for you but not for  
others: just commit the right fix

You cannot revert file add and file remove

So imho use reversal only for wrong use of cvs (technically wise or  
policy wise), not for nonworking code

[...]
> II. POLICY / RULES:
> ===================
> 
> 1. You must not commit code which breaks MPlayer! (Meaning unfinished
> but
>    enabled code which breaks compilation or compiles but does not
> work.)
imho committing uncomplete/partial working code (that compiles  
correctly) that implements a new feature (so it doesn't break current  
features) is ok

> 
> 2. You don't have to over-test things. If it works for you, and you
> think it
>    should work for others, too, then commit. If your code has  
> problems
>    (portability, exploits compiler bugs, unusual environment etc)  
> they
> will be
>    reported and eventually fixed.
keep it this way, for exotic hardware or features nobody cares about  
it's better to trust the patch committer than let the patch unapplied  
(and maybe ask him if he wants to take mantainership of that code, if  
currently unmantained).

> 
> 3. You can commit unfinished stuff (for testing etc), but it must be
> disabled
>    (#ifdef etc) by default.
same as 1, why duplicate it?

> 
> 5. Do not commit changes to the build system (Makefiles, configure
> script)
>    which change behaviour, defaults etc, without asking first. The
ok
> same
>    applies to compiler warning fixes, trivial looking fixes and to
> code
>    maintained by other developers. We usually have a reason for doing
> things
>    the way we do. Send your changes as patches to the mplayer-dev-eng
> mailing
>    list, and if the code maintainers say OK, you may commit. This  
> does
> not
>    apply to files you wrote and/or maintain.
this is very similar to 8, see comments there

> 
> 6. We refuse source indentation and other cosmetic changes if they  
> are
> mixed
>    with functional changes, such commits will be rejected and  
> removed.
> Every
>    developer has his own indentation style, you should not change it.
> Of course
>    if you (re)write something, you can use your own style... (Many
> projects
>    force a given indentation style - we don't.) If you really need to
> make
>    indentation changes, separate them strictly from real changes.
Remark _really_
we should not encourage people to commit cosmetics after every patch

> 
>    NOTE: If you had to put if(){ .. } over a large (> 5 lines) chunk
> of code,
>    do NOT change the indentation of the inner part (don't move it to
> the right)!
>

> 8. If you apply a patch by someone else, include his name and email
> address in
please add some "antispam" to mail address, don't put them unchanged on  
the list; list is archived in a lot of public web pages.
>    the CVS comment! Do NOT commit patches for other developer's code
> (code not
>    maintained by you) without his permission! If he didn't commit -  
> he
> probably
this should apply only to code with an active mantainer if patch is a  
fix, and probably also if it adds an useful feature.
If a mantainer disappears for months we can't stop developement.
Obviously you need to understand the code and the patch, not apply it  
blindly
That's also very similar to 5

>    has a reason! Send an answer to mplayer-dev-eng (or wherever you
> got the
>    patch from) saying that you applied it.

> 
> 9. You will have write access to DOCS/. If you are unsure about
> changes you
>    make to the documentation, send a patch to mplayer-docs, the
> documentation
>    maintainers will review and commit your stuff.
Strangely you forgot about this :)
when someone submit a patch that changes/extends command line syntax,  
ask him to send a patch also for the man page.
but imho do not reject an useful patch just because author did not  
submit patch for docs

> 
> 10. Subscribe to the mplayer-cvslog mailing list. The diffs of all  
> CVS
> commits
>     are sent there and reviewed by all the other developers. Bugs and
> possible
>     improvements or general questions regarding commits are discussed
> there. We
>     expect you to react if problems with your code are uncovered.
as asked in a previous message, i think subscribing to advuser can be  
suggested, that list gets about 10 msg/month
should devels subscribe to -announce?
[...]

Hope i dodn't forgot anything.

Ciao,
 Roberto