CVS access policy, branching/tagging, code review, etc.

[Egbert Eich]

Mike A. Harris writes:
 > This topic has been asked a lot recently by people in IRC and
 > elsewhere, but nobody is quite sure yet what exactly the official
 > answers are, or who in particular decides.  Since this is 
 > intended to be an open community project, I figured the best 
 > place to post my query was probably here for open discussion, 
 > rather than to any specific individual(s).
 > 
 > We need to put together guidelines for how:
 > 
 > - Does one gain CVS write access to the repository, who needs to 
 >   be contacted, what steps need to happen before access is 
 >   granted (if any), who decides, etc.

I've thought about this in the past quite a bit.
There are many people who come along and say:
If you give me access to your tree I will do this-and-that.
You give them access that's the last you hear from them (in
the best case). In the worst case they start screwing up
a few things and then vanish.
So my conclusion was that one should either be able to show
some code or at least provide a rough idea of his plans 
and/or be able to demonstrate his abilities. Further the
person needs to have read and understood the rules.
Generally with a multi tiered commit access model this person
can get permission to create a new project (branch) or get
permission to write to a certain project (branch) by the owner
of this project. Therefore generally the 'owner' of a branch
should be contacted who then decides and adds this person
to the list of committers on his branch. If this branch
happens to be the main trunk it's more difficult. Some projects
put the release manager of the next release in charge.

 > - what the official do's and dont's are in the tree

Part of them can be directly derived from the policies others
are related to system limitations like:
- CVS doesn't allow to have a directory name which later on
  turns into a file of the same name. 
  If somebody messes up here this can only be fixed by manipulating
  the repository by hand which should be avoided under all 
  circumstances.
- We need to take into account that some systems cannot distinguish
  between upper and lower case characters.


 > - what happens if someone commits something non-trivial without 
 >   proper code review, which others believe should not have been 
 >   committed, or which horribly breaks the tree, etc.

This can happen by accident. I think peer pressure will help to avoid 
this. First thing would be to ask the person to back out the changes.
We should be able to notice if the person does this by accident or 
is plain stupid. If nothing helps this person can get kicked. Usually 
there is a pretty good consensus on this.

 > - A specific guideline for the naming of CVS tags, branches, etc.

Yes. Currently we have a lightweight procedure:
If you have the project FOO you create a branch base: FOO-BASE
an branch FOO and once you merged in the trunk you mark this version
FOO-MERGE. You can move FOO-MERGE from merger point to merger point.
(During merger you may have to make a temporary tag FOO-MERGE1 which
will be replaced by FOO-MERGE after the merger is done).

 > - What type of checkins are permitted into particular branches

That should be coordinated with the branch 'owner'.

 > - What happens if someone consistently violates the accepted 
 >   written or defacto policies and shows disregard for the 
 >   established guidelines repeatedly.

Person gets access revoked.

 > 
 > As well as some simple guidlines on how:
 > - to properly tag the tree with a particular tag
 > - to branch properly
 > - to import external code into the repository such as 
 >   Mesa/Xft/Xrender/etc. (ie: tag/import/tag/cleanup, or whatever)
 > 
 > And for code review:
 > - What types of fixes/checkins are considered ok to do without 
 >   review (and on which branches, and during what phases of 
 >   development).

Depends a little bit on the person. 
General rule: if you are not sure: ask!
Other projects have different levels of committers.
1. ByReview committers 2. Project committer. 3. global committers.
Everybody is allowed to commit obvious things everywhere, apart
from this it depends on the status what the person is allowed to
do. 

For releases there should be a release branch. This should be off
limit for everyone except the people who are involved in the release
process.

 > - What types of changes ALWAYS require review either via bugzilla 
 >   filing or email list posting and having one or more people 
 >   "ACK" (acknowledge) the fix is acceptable for the tree, or 
 >   "NACK" it indicating it is unacceptable, and providing a reason 
 >   why.
 > - What types of changes are good ideas, or worth exploration in 
 >   the future, but are not acceptable changes for the current 
 >   goals of releasing a stable release as soon as possible, such 
 >   as Phase2/Phase3 type of work, source code beautification, 
 >   lindent, etc.
 > 
 > I've been putting together a document for some of this, with 
 > plans on posting it when it's more complete, however I think now 
 > is a good time to harvest some thoughts from the other 
 > participants as well.  I can then use this feedback in my first 
 > draft, and when it's ready, I can post it to the list.
 > 
 > Constructive comments and feedback are appreciated.
 > 

I basically like the DRI projects policies.
Kevin and Ric worked them out so maybe we can ask them to
help us and use the DRI policies as a basis.

Egbert.