Commit Conventions

The following are notes on the WarFoundry conventions for committing to source control. They should, in general, be quite similar to other systems using Subversion and Trac to integrate tickets with source control.

Code quality on commit

There are three priorities when committing code:

1. the code compiles
2. the code works
3. the commit is as small as possible

Code must compile

All code commits must comply with the first rule unless completely unavoidable and must compile. Ideally someone should be able to check out any revision and build it, so the code must compile. The other advantage of making sure that code compiles is that it stops the situation occurring where a second commit isn't made for a while (due to network issues or time constraints) that would have allowed the code to compile, ensuring that everyone can update without fear of things breaking

Code should work

Code commits should also commit working code. This isn't essential, as there are situations where a feature may compile but its functionality isn't complete. If it isn't complete (or if, for example, a listener hasn't been attached to a button) then there should be a ticket open to track the work being done.

Commits should be small

The final requirement is not essential but is very useful when possible as it allows change tracking to be made at a more granular level. It also reduces the amount of work you might have to do to rebuild your changes if you do something wrong as you have a more recent point to go back to.

Committing little and often is easier and more useful than doing a big commit.

Commit Messages

As part of the integration between Subversion and Trac to link bug reports/tickets to source code changes, we have a couple of fairly standard hooks on the repository. For information on conventions when referencing tickets and revisions, please read the Trac Conventions page.

Format

The format for a commit message is up to the individual, but a convention that seems to work well in a corporate environment that should transfer well is to use something like:

Re #3: Ticket title (or brief description similar to the title)
 * Some grouping of changes
 * Some other group of changes
 * Another group of changes

This format works well in both Subversion and Trac (which would munge everything on to a single line without the bullets) and using a descriptive overview of changes rather than the detail means the commit message abstracts away the detail. If someone wants to know exactly what is involved in the commit that "Added LGPL headers to all files" then they can look at the revision details and the file differences.

Pre-commit hook

When you try to commit code, but before the code is actually committed, the system checks your commit message for Trac ticket references (e.g. #1 or ticket:2). You must reference at least one ticket in all commit messages or the commit will be rejected.

This is generally a good thing as all code should normally be written because of a ticket. If there isn't a ticket for the feature or the bug then why is the code being written? There are, however, times when you've closed a ticket and found a bit of code that wasn't checked in (e.g. here), or when a very short fix needs to be made where a ticket is more effort than the fix. In these situation you should reference the closed ticket (if applicable) and use the keyword "no-open-ticket" (with the hyphens). Excessive and unnecessary use of the keyword should be avoided.

Post-commit hook

So that you don't have to update Trac manually, we also have links from the commit message to referenced Trac tickets. These let you tie commits to a ticket, leave comments on the ticket and close the ticket without accessing Trac directly and copying what you just sent to Subversion.

Adding comments to a ticket

References to tickets using any of "references", "refs", "addresses", "re" or "see" followed by a Trac ticket link (e.g. #1 or ticket:2) leaves the referenced ticket in its current state and appends the commit message.

There are no hard and fast rules, but in general "re #ticket" is generally sufficient.

Closing a ticket

If the commit resolves the issue a ticket refers to then the ticket can be closed with the commit message. This is done in a similar manner to commenting on a ticket by using "close", "closed", "closes", "fix", "fixed" or "fixes" followed by a Trac ticket link (e.g. #1 or ticket:2). This will add the comment to the ticket and close it.

There are no hard and fast rules for which word to use when closing tickets, but there are some rough guidelines that may make sense. "Fixes" seems to be the most appropriate keyword to close bugs/defects since they are broken bits of functionality. Tasks and enhancements seem more like types of tickets that get "Closes" since they're not broken, they're just an issue that needs resolving.

Handling multiple tickets

In general commits should be made against a small number of tickets, but ​multiple tickets can be updated with a single commit message through lists and the word "and".