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

Posted by Eduardo Pérez on
<blockquote> what would you propose then? That people have to commit on both the release and HEAD themselves?

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,

These are the conflicts I'm referring to.

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

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.

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.

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)

Posted by Jeff Davis on
Here are the problems I see with your approach:
  • People have to have both a 5.1 and HEAD checkout (a lot of people probably don't keep both around).
  • People have to know how to merge with CVS (a lot don't know how to do it cleanly).
  • It's non-trivial to do the merge for multiple files and it's easy to miss one or two.
  • If you are merging just your changes it can have conflicts that arise from past changes on HEAD or the release which were unmerged (which if we continue to do bulk merges is not as much of an issue).
  • If someone misses merging a change how do we know?

With the current rules we are very confident that nothing gets lost with the primary risk being that a conflict gets resolved the wrong way, at the expense of having HEAD be somewhat out of date with respect to the release branch bug fixes.

Under your suggested methodology, the risk would be code gets lost, individuals merging make the wrong decision merging conflicts.

I would dispute the assertion that the overall error rate would decline if people resolved their own conflicts -- I have been merging roughly monthly for about a year, with a typical merge affecting ~800 files and there have been about 10 conflicts that I am aware of being done the wrong way of 1270 commit sets, affecting 7500 files). Depending on how you calculate it that error rate is somewhere between 1% and .1%; I would claim that if you had the 30 people who had done those commits also responsible for the merges on each commit set, that the error rate would be much higher (and I guarantee we would have lost code/bugfixes along the way).

In order for your approach to work better someone would have to take responsibility for making sure that peoples commits were merged properly and if not remind them to do the merge (and teach people how to properly do merges). And also periodically make sure that no code had been lost.

I am happy to do the merges with the commit guidelines we have now and I am confident in claiming we are not losing code or bug fixes. Under your proposed solution I would not be willing to take that QA role since a) it would be a lot more work, and b) I would not be at all confident that code would not be slipping through the cracks. When I said it would take more resources that's what I meant.

This has come up before and I still don't think there are that many instances of things fixed in 5.1 which have not been fixed on HEAD. If you want to merge to HEAD more frequently then I am only to happy to have you carry out a merge (and tell you how I have been doing the merges to date so it's all done consistently).

If you want to propose changes to the commit guidelines then submit a TIP and lobby the OCT members to vote for it. I would say that unless you found the resources to insure your approach delivered better overall quality that what we do now I personally would vote no.

Posted by Roberto Mello on
I agree with Jeff. Our current approach is not perfect, but it's the best we've identified so far.

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.


Posted by Joel Aufrecht on
Eduardo, can you explain this a bit more for me:
> For core packages, bug fixes should be committed on the 
> current release branch whenever applicable.

The problem with this one is that some developers have to 
develop with old code in the stable branch because HEAD 
contains bugs. I see many people on the lists that have 
these problems.
I think most developers should be developing against 5.1, not HEAD. The rules about bug fixes vs new features only apply to core, not to the other packages. (I know all this could be clearer - it's taken me about two years to understand even as much as I do now, which is still only a subset of the problem space and a smaller subset of the solution space - and I'm going to work on some docs this week.) The only time you should be working on HEAD is if you are doing development directly on core packages. This approach should get you all of the benefits - you can still do whatever you want to your packages. If you are working on the standard packages (file-storage, forums, etc), you can still do minor or even major releases, and you know your releases will be available for most other users much sooner than if they depended on HEAD core.
Posted by Joel Aufrecht on
I've updated TIP #61 with the amendments from TIP #70.  I took the liberty of relabelling the new rules 5 and 6 as 5.1 and 5.2, and moved some of the reasons out to the Reasons section for easier reading.