Latest revision as of 20:59, 23 November 2006

Think twice before committing.

Committing something to SVN has serious consequences. All other developers will get your changes once they are in SVN, and if they break something, they will break it for everybody. All commits will be publicly available in the SVN repository forever.

On the other hand SVN allows one to revert changes, so it's possible to recover from mistakes. This is relatively easy for commits to single files but it can also be a significant amount of work for bigger changes.

The baseline is: Be aware of the consequences of your commits. Take time to think about them before committing.

Never commit code that does not compile.

Compile the code and correct all errors before committing. Make sure that newly added files are comitted. If they are missing your local compile will work fine but everybody else won't be able to compile.

You certainly should make sure that the code compiles with your local setup. You should also consider what consequences your commit will have for compiling with the source directory being different from the build directory. It would also be nice if it would compile with the --enable-final option, but we don't explicitly support that.

Be especially careful if you change the build system, i.e. Makefiles

Test your changes before committing.

Start the application affected by your change and make sure that the changed code behaves as desired.

Run regression tests if available ("make check").

Double check what you commit.

Do a "svn update" and a "svn diff" before committing. Take messages from SVN about conflicts, unknown files, etc. seriously. The "svn diff" will tell you exactly what you will be committing. Check if that's really what you intended to commit.

Always add descriptive log messages.

Log messages should be understandable to someone who sees only the log. They shouldn't depend on information outside the context of the commit. Try to put the log messages only to those files which are really affected by the change described in the log message.

In particular put all important information which can't be seen from the diff in the log message.

The code you commit must adhere to the Adempiere coding policies.

This includes security (shell quoting, buffer overflows, format string vulnerabilities), binary compatibility (d pointers), i18n, usability, user interface style guide, (API) documentation, documentation and definition of memory management and ownership policies, method naming, portability issues and license notices.

These policies are defined separately. If in doubt ask on the mailing list.

Respect special commit policies set by the release plans.

Respect the policies of application and library maintainers, and consult with them before making large changes.

Source control systems are not a substitute for developer communication.

When you plan to make changes which affect a lot of different code in SVN, announce them on the mailing list in advance.

Changes which affect a lot of code in SVN, like making use of a new feature in the libs, might break other code even if they look trivial, e.g., because an application must also compile with older versions of the libs for some reasons. By announcing the changes in advance, developers are prepared, and can express concerns before something gets broken.

Don't commit changes to the public API of libraries without prior review by other developers.

There are certain special requirements for the public APIs of the Adempiere libraries, e.g., source and binary compatibility issues. Changes to the public APIs affect many other Adempiere developers including third party developers, so requiring a review for these changes is intended to avoid problems for the users of the APIs and to improve the quality of the APIs.

Take responsibilty for your commits.

If your commit breaks something or has side effects on other code, take the responsibility to fix or help fix the problems.

Don't commit code you don't understand.

Avoid things like "I don't know why it crashes, but when I do this, it does not crash anymore." or "I'm not completely sure if thats right, but at least it works for me.".

If you don't find a solution to a problem, discuss it with other developers.

Don't abuse your SVN account to push in changes with which other developers disagree.

If there are disagreements over code changes, these should be resolved by discussing them on the mailing lists or in private, not by forcing code on others by simply committing the changes to SVN.

If you commit bugfixes, consider porting the fixes to other branches.

Use the same comment for both the original fix and the backport, that way it is easy to see which fixes have been backported already.

If you fix bugs reported on the bug tracking system, add the bug number to the log message.

In order to keep the bug tracking system in sync with SVN, you should reference the bug report in your commits, and close the fixed bugs in the bug tracking system.

This doesn't mean that you don't need an understandable log message. It should be clear from the log message what has been changed without looking at the bug report.

There are rules for where to place release tags and branches in the repository.

Official Adempiere branches and release will be created by the release Adempiere coordinator in the branches/Adempiere and tags/Adempiere directories, scripts will ensure that this dirs are protected.

Developers should place all branches which are aimed to be released in branches/<appname> and name them like the release, e.g. branches/<appname>/1.5. For alle release tags, tags/<appname> is the right place.

All temporary working branches (which should be deleted again after the work has ended) should be located in branches/work with some name describing both which part of Adempiere (or which application) is branched and which work is done there. Bad idea would be something like branches/work/make-it-cool.

Don't add files generated by standard tools to the repository.

Files generated at build shouldn't be checked into the repository because this is redundant information and might cause conflicts. Only real source files should be in SVN. An exception to that are files generated by tools that would be an unusual requirement for building Adempiere.

Don't use SVN keywords like Id or Log in the source files.

These tags cause unnecessary conflicts when merging branches and don't contain any information which wouldn't be available in the SVN repository anyway.

Make "atomic" commits.

SVN has the ability to commit more than one file at a time. Therefore, please commit all related changes in multiple files, even if they span over multiple directories at the same time in the same commit. This way, you ensure that SVN stays in a compileable state before and after the commit.

Don't mix formatting changes with code changes.

Changing formatting like indenting or whitespaces blows up the diff, so that it is hard to find code changes if they are mixed with reindenting commits or similar things when looking at the logs and diffs later. Committing formatting changes separately solves this problem.

If your commit causes user visible GUI changes, add the GUI keyword to the log message.

Addition of the keyword "GUI" to the log message makes sure that the documentation writers get notified of your changes.

Appendix: Special keywords in SVN log messages

When you commit changes to SVN you will be asked for a description of your commit. There are several special keywords defined that you can use in this description. These keywords are always uppercase and followed by a colon. Each keyword should be placed on a line of its own. The following keywords can be used:

BUG: <bugnumber>

Marks the bug as closed by CC'ing the commit message to <bugnumber>-done@bugs.adempiere.org This keyword will also be used to automatically extract entries for the release changelog.

CCBUG: <bugnumber>

CC's to the bugreport by sending mail to <bugnumber>@bugs.adempiere.org

CCMAIL: <email-address>

CC's to the given e-mail address.

SVN_SILENT:

Marks the commit message "silent" by adding "(silent)" to the subject of the mail to allow filtering out trivial commits. Use this tag carefully and only for really uninteresting, uncontroversial commits.

FEATURE: [<bugnumber>]

Marks the feature as implemented by CC'ing the commit message to <bugnumber>-done@bugs.kde.org. This keyword will also be used to automatically extract entries for the release changelog, so it makes sense to use it for new features even if you don't have a bugnumber for the feature.

GUI: Indicates a user visible change in the user interface. This is used to make the documentation team aware of such changes.