Forum OpenACS Improvement Proposals (TIPs): TIP #61: (Approved) Guidelines for CVS committers

Proposal

CVS committers are trusted to make direct changes to the code base for OpenACS and its packages.

1. Which branch to use
   1. For core packages, new features should always be committed on HEAD, not to release branches.
   2. For core packages, bug fixes should be committed on the current release branch whenever applicable.
   3. For non-core packages, developers should work on a checkout of the release branch of the lastest release. For example, if OpenACS 5.1.0 is released, developers should work on the oacs-5-1 branch. When oacs-5-2 is branched, developers should continue working on oacs-5-1 until OpenACS 5.2.0 is actually released.
   4. The current release branch is merged back to HEAD after each dot release.
2. New packages should be created in the /packages directory and the maturity flag should be set to zero. (This is a change from previous policy, where new packages went to /contrib/packages)
3. Code
   1. Only GPL code and material should be committed to the openacs.org CVS repository
   2. Code should only be reformatted when functionality is changed, e.g. when you change control flow and reindent to reflect it.
   3. Database upgrade scripts should only span one release increment, and should follow Naming Database Upgrade Scripts.
   4. Database upgrade scripts should never go to the release version, e.g., should always have a letter suffix such as d1 or b1.
   5. CVS commit messages should be intelligible in the context of Changelogs. They should not refer to the files or versions.
   6. CVS commit messages and code comments should refer to bug, tip, or patch number if appropriate, in the format "resolves bug 11", "resolves bugs 11, resolves bug 22". "implements tip 42", "implements tip 42, implements tip 50", "applies patch 456 by User Name", "applies patch 456 by User Name, applies patch 523 by ...".
4. When to TIP
   • A proposed change must be approved by TIP if
     • It changes the core data model, or
     • It will change the behavior of any core package in a way that affects existing code (typically, by changing public API), or
     • It is a non-backwards-compatible change to any core or standard package.
   • A proposed change need not be TIPped if
     • it adds a new function to a core package in a way that:
       • does not change the backwards-compatibility of public API functions.
       • does not change the data model
       • has no negative impact on performance
     • or it changes private API.
     • or it is a change to a non-core, non-standard package
5. Tags
   1. When a package is released in final form, the developer shall tag it "packagename-x-y-z-final" and "oacs-x-y-compat". x-y should correspond to the current branch. If the package is compatible with several different core versions, several compat tags should be applied.
   2. When OpenACS core is released, the openacs-x-y-z-final tag shall be applied to all compat packages.

For example, adding a new API function wouldn't require a TIP. Changing an existing API function by adding an optional new flag which defaults to no-effect wouldn't require a TIP. Added a new mandatory flag to an existing function would require a TIP.

Reasons

We don't currently have clear standards for committing code.

Rule 1.3: First, this ensures that developers are working against stable core code. Second, it ensures that new package releases are available to OpenACS users immediately.

Rule 3.4: If an upgrade script ends with the final release number, then if a problem is found in a release candidate it cannot be addressed with another upgrade script. E.g., the last planned upgrade script for a package previously in dev 1 would be upgrade-2.0.0d1-2.0.0b1.sql, not upgrade-2.0.0d1-2.0.0.sql. Note that using rc1 instead of b1 would be nice, because that's the convention with release codes in cvs, but the package manager doesn't support rc tags.

Rule 5.1: Reason 1: The packagename tag is a permanent, static tag that allows for future comparison. The compat tag is a floating tag which is used by the repository generator to determine the most recent released version of each package for each core version. This allows package developers to publish their releases to all users of automatic upgrade without any intervention from the OpenACS release team.
Reason 2: The compat tags allows CVS users to identify packages which have been released since the last core release.
Reason 3: The compat tag or something similar is required to make Rule 6 possible.

Rule 5.2: This allows OpenACS developers who are creating extensively customized sites to branch from a tag which is stable, corresponds to released code instead of development code, and applies to all packages. This tag can be used to fork packages as needed, and provides a common ancestor between the fork and the OpenACS code so that patches can be generated.

Disadvantages

Implementation

Edited after initial publication to reflect Jeff's clarifications of comment formatting; I also made the formatting a bit more verbose for more reliable parsing

Edited to reflect changes from TIP #70.

Thanks Joel!

One question: does this mean we're going to move a lot of the current /contrib/ packages into the /packages directory? Or is this only for new packages?

Jade: yes, we should roll contrib over after we ship 5.2 with working maturity level.

"close" is shorter though :)

The intent is to avoid mixing reformatting with changes and by "functionality" the meaning was control flow so if you went from:

 
blah
bleh
foo

blah 
if {![zim]} { 
    bleh
    foo
}

I agree that you want to seperate commits to bring code into conformance with formatting conventions from actual code changes and it's something we should add as part of the guidelines. On the other hand, recognize that reformatting comes with quite a cost to people maintaining seperate archives for their sites, as it can mean tremendous numbers of conflicts.

I did a little checking to confirm what we already knew which was that tcl is overwhelmingly indented 4 spaces (something like 15:1 versus 2).

I think if we wanted to reformat code to bring it into compliance with standards (I think we need to be clearer about what those are) there are good times to it. Best near term would be when 5.1 is pretty much stable and 5.2 is not yet branched. That should be pretty soon (probably after the dotlrn bug bash would be a good time).

<blockquote> The current release branch is merged back to HEAD after each dot release.
</blockquote>
This doesn't work as well as expected as there are usually conflicts and they don't get resolved very well.

Anyway this is just my humble opinion. You are the ones that approve this proposals.

Yes, that's what I'm proposing. Since the bugfix commiter has the knowledge of the bug and the bugfix he is the best one to commit on both branches instead someone that doesn't know very well how the bugfix work at release commit from the branch to HEAD.

<blockquote> I am not sure this is really accurate; there have been some problems with merge conflicts,
</blockquote>

These are the conflicts I'm referring to.

<blockquote> If your proposal is that people are responsible for merging when they commit bug fixes,
</blockquote>

What's the problem with committing the bugfix in both branches?

<blockquote> I think the short answer is that it's not administratively tenable to do it that way unless someone steps forward to take responsiblity for auditing commits to make sure that bug fixes do get rolled forward to HEAD.
</blockquote>

You can still merge bugfixes to HEAD.
But people should commit on both branches.

<blockquote> The way we do it now may not be ideal but I think it's the best solution with the resources we have now.
</blockquote>

What additional resources are needed in my proposal?

At least with my proposal people can develop with HEAD.

What do other people think?
Should this be posted to OpenACS Development? (for other developers to know)

Jeff has been doing a great job with CVS maintenance and merging, and has proven that it works well.

The problem that you are really trying to address is the disparity between the stable and the HEAD branches. The compromise I can see would be to do more frequent merges. But as Jeff has pointed out, this is more work. If you are willing to take on that work, and can convince Jeff that you can do it well, I'm sure other OCT members would follow.

-Roberto