views:

304

answers:

7

My boss announced yesterday a new commit policies for checkins into the repository. This policies are valid for commits into head/trunk and branches.
A commit message must have the following items:

  • Reason (Bug ID,Project ID, or non functional change)
  • Name of the reviewer

After the commit we also have to create a change blog entry in our CMS.

I'm not a big fan of this commit policies, because I don't normally need a reviewer when I am doing new or experimental stuff in a non-productive-branch.

Do you have any commit policies do you have to follow?

I think it is a good idea to change the productive-branch only due to a Bug Report, but commits into the development branches should be less restrictive.

+1  A: 

A reviewer seems pointless for the reasons you mentioned, because not everything needs to be reviewed by others.

In the past the only commit policy we had (where I used to work) was to include a comment indicating what you changed and why, but that's more common sense than anything else.

Ian Devlin
+2  A: 

Commit early and commit often.

We actually use /trunk as development and tags to branch different releases. Only structural intrusive changes go in /branches.

We actively use tags for production and acceptation releases, so we can go back in time easily. Anything committed in the trunk should only have a message describing what the commit changed or added briefly.

I'm not a big fan of using the message space to link with Bug ID's it still requires a lookup for the ID in which case you could also look it up in the bug tracking software and close it there, which to me is about the same effort.

Not to say i dont like any svn integration: - We use more goodness of automated nant scripts to make releases which branches them in /tags - svn props actually store our version numbers :p. - hook scripts for email notification and message logging (great for copy pasting release notes).

Martijn Laarman
+2  A: 

We have a number of policies, which are enforced via an in-house plug-in to Visual Studio. We check that code compiles and that unit tests have be run successfully. At the moment we also check code coverage and issue warnings for code which doesn't have enough tests. We also do various consistency checks and verify that an appropriate task is present in our change management system in order to provide traceability for all changes.

The advantage of tool support is great, as it is not really up to people to respect the policies, but obviously there's a drawback as well as these checks take time to run. However, with many developers it is hard to enforce standards without proper tool support.

Brian Rasmussen
+1  A: 

A common commit policy is to associate a bug ID to the commit to trunk as a justification. Sometimes, version control and bug tracking systems are configured to enforce this policy.

mouviciel
A: 

My commit message include a short describtion what i have implemented or changed in the classes.

The bug number and additional describtions i put in the commentation above the new code. IDs inside the commit messages we put when we merge changes into a tagged branche.

Every night a automatic build checks the different features and products too get sure that the code base is stabil.

But in the end i think you can not have too many describtions for new or changed classes but too many policies you have to do before a commit. The name of the reviewer is something which i would not put into the commit message.

Think about that you sometimes have to undestand your code which you have implemented 2 years ago. And then you are happy about commit messages which are not like "Update after debugging".

Markus Lausberg
+1  A: 

Our commit policy sounds a bit like yours, only we don't enforce it on task branches (where a task branch is like a developer's sandbox for experimenting).

Our commit comments must include either a change control ID (new feature, enhancement) or an issue ID (bug fix). You must also include a brief explanation as to why you made this change; version control tracks the who, what, when and where.

Patrick Cuff
A: 

We have branches for every released major version of the software that is still actively supported. Checking into any of these branches requires a bug ID - this is enforced by scmbug, which will not only check that the comment is prefixed by the bug ID, but will also look up this bug in the bug database, ensure it is assigned to the committer, and potentially check other criteria (e.g. that the "fix in branch" field is the branch being committed to).

One of the products has more potential to fail in embarrasing ways, and checkins to this require not just a bug ID but a code review as well. However, the criteria for the code reviewing is handled in our bug database - we have custom fields for this and the bug cannot be accepted and closed until it has been reviewed. To me this works from a conceptual level - it's probably better to check code that is believed to work into the repository un-reviewed, then reopen the bug and change it if necessary rather than hold off on committing until you're sure it is ready for release.

Other than that, there's no explicit policy for the trunk (though of course the general tenets of checking in often without breaking the build, including good descriptive commit messages, checking in units of work atomically still apply).

Andrzej Doyle