← Back to team overview

mugle-dev team mailing list archive

Re: Proper revision control practices

 

Hi guys,

Again, there is a serious problem with the revision control as of trunk r69.
I will not fix up the history this time (since we aren't really expecting
the client side to do much at this stage), but we should fix this asap. I
have opened a new bug, #736029<https://bugs.launchpad.net/mugle/+bug/736029>
.

I am just a bit dismayed that the branch was merged while it did not
compile. The point of trunk is that it is always stable. Any commits made to
trunk should compile (both the server *and client*) and work the same or
better than the previous commit. Yes, there is the occasional mistake where
it breaks the build, but this should never happen when merging a branch.

The whole point of a branch (and the very reason we decided to start this
one) is to do work which will potentially break things without causing
trouble for the trunk (so the trunk will continue to build and work fine,
even when the branch is experimental or broken). So there is very little
point to using a branch if the branch is merged to trunk in a broken state.

The fact that the client never compiled during the entire history of the
branch (due to the java.io.File problem) wasn't really a concern of mine,
since after all it was a branch. But I did expect that to get fixed before
the branch was merged.

We didn't have this before, but I think we should introduce a review process
for merges. Many projects require a review before every commit -- that is
too much work for us. But standard practice is to have a review before a
branch is merged, because branches are much larger than normal commits and
might break more things. So I think from now on we should have this policy.

Note in the branch page in Launchpad, there is a button called "Propose for
merging". This will take you to a simple form you can fill out to explain
what the branch does and what you want the reviewer to look for. This will
then notify the group, and create a review form for someone else to fill
out. If you can review someone else's branch, that would be good, otherwise
I can have a look. Reviewing should involve compiling the branch and testing
its new features to make sure they work properly. You may then leave
comments on the review page and choose to accept or reject the merge. You
should only merge the branch into trunk once another developer has accepted
the merge.

I think this process is a good compromise between no reviews and full
reviewing of every commit, and should avoid such breakages in the future.

References