Title: Coding Guidelines for Thuban
Author: Bernhard Herzog <bh@intevation.de>
Last-Modified: $Date: 2004/12/11 02:03:47 $
Version: $Revision: 1.3 $


Introduction

    To keep the Thuban code maintainable all code should adhere to the
    guidelines outlined below.  The guidelines currently cover stylistic
    issues (e.g. rules for source code formatting) as well as Python
    programming hints and rules for using CVS and making patches.


Source Formatting

    Python and, where applicable, C/C++ source code should be formatted
    according to these rules:

    - All lines should be at most 79 characters long

    - Put spaces around binary operators and after commas, colons and
      semicolons.  Do not put spaces before commas, colons and
      semicolons and don't put them after opening or before closing
      parentheses, brackets and braces.

    - Use only spaces for indentation.  Each indentation level is four
      spaces.

    - Every class, function and method should have a doc string.  The
      doc string should start with a brief one-line description of the
      class or method.  If more explanations are needed add an empty
      line and one or more paragraphs.

    - Put imports at the top of a module.  Put the more fundamental
      import statements first, and the more Thuban specific later.
      E.g. imports from the standard library come first, then import
      statements for third-party modules, then Thuban imports.  Imports
      from the same Thuban sub-package come last.


Python Programming

    - Thuban must remain compatible with Python 2.2.

    - Do not use "from module import *"

      This form of the import statement leads to code that is hard to
      maintain for several reasons:

      - When the module's contents change, the names bound in the code
        that executes the import statement change as well and might
        accidentally override python builtins or names already bound in
        the module

      - It's hard to find out which of the objects in the imported
        module are actually used by the importing code.  It's especially
        hard to find out whether the import is still needed if the code
        has changed.

    - Do not check the type of parameters

      Functions and methods should be coded to interfaces, not types, so
      checking whether an object passed as a parameter to a function is
      of a certain type or an instance of a particular class is almost
      never a good idea.

    - Use "obj is None" when testing for None not just "obj".  The
      object in question might be false without being None.

    - Method names start with lower case letter.

      The main reason is that we should, in the long term, adopt the more
      common naming styles used in python code.  For Thuban this basically
      means not to start method names with upper case letters. For Thuban this
      would be a substantial change and even though it would be easy to retain
      the old method names for a while for backwards compatibility it's not
      something that should be done soon.  However, new classes should
      follow this rule.

Test Suite

    Thuban has a fairly comprehensive test suite in the test/
    subdirectory.  The test suite is only useful if it is kept up to
    date and if it is run often.  Therefore:

    - Before a checkin, run the *entire testsuite*.  Yes, all of it.
      Even if you think your change won't break anything.

    - New functionality and bug fixes should have corresponding tests in
      the test suite.

    - The tests should use only public interfaces.

    - Write the tests before writing the code.

    - It should be possible to run the test suite without an X server
      being present.


CVS

    - One can't say this often enough: Before a checkin, run the *entire
      testsuite*.  Yes, all of it.  Even if you think your change won't
      break anything.

    - All commits should be described in the ChangeLog file.  The easiest
      way to do that is to write the ChangeLog entry first and use that
      as the commit message.

      For emacs users: Use emacs' CVS support (e.g. `C-x v =' in a
      modified file's buffer) to get a diff and then use `C-x 4 a' at
      appropriate places in the diff to start the ChangeLog entry.

    - Try to make small self contained commits.  In particular, if
      during the work on a more complex change you discover a bug,
      commit the the fix to that bug separately with a separate
      ChangeLog entry.



Patches

    - When you produce patches please try to produce them as patches
      against a current CVS version.  A patch against code from a
      tarball is often OK too but can make it more difficult to test a
      patch if e.g. the files in CVs have changed considerably.

    - Please make context diffs, i.e. use the -c option of diff or cvs
      diff.  The default output of diff is not suitable for a patch.

    - Treat a patch like CVS commit.  That's what it will end up as if
      it is accepted, so if you want to increase the chances that it
      will be applied, please try to make the work easier for us, and
      make sure the test suite still works, add new tests if your patch
      adds functionality or fixes a bug and write a ChangeLog entry.
