## 1. General rules applied in the code base

### I. Introduction

These are general rules independent of the coding  language.

### II. Catkin Package and Repository Naming

- Name: lower_cases_with_underscore.
- One repository per catkin package.
- Same name for package and repository.

### III. Versioning

Use semantic versioning when versioning packages. Short summary:

- Given a version number MAJOR.MINOR.PATCH, increment the:
- MAJOR version when you make incompatible API changes,
- MINOR version when you add functionality in a backwards-compatible manner, and
- PATCH version when you make backwards-compatible bug fixes.

Regarding formatting of pre-releases (alpha, beta, rc, ...) consider PEP440,
especially when versioning Python packages (note that there is no general
conflict with semantic versioning, only the format is a bit different).

### IV. Contribution Guidelines

#### IV.1. Code revision:

To ensure some level of code quality, all modifications have to be reviewed
before they can be merged. This is done via merge requests in GitLab
(on GitHub it is called pull request but the concept is the same).

The procedure for adding a change is as follows:

- When creating branches, use your name as a namespace,
    e.g. if you are John Doe name you branch *jdoe/fix_segfault_demo1*.
    This is to avoid confusion with branches of other people and to indicate who
    is responsible for that branch. It is not allowed to push branches without
    such namespace. Moreover use a **meaningful branch name**!
- Test all your changes and update documentation.
- When finished, create a merge request to the master branch of the main
    repository.
- Add one or more of the maintainers as reviewers. The reviewers will review and
    maybe request changes.
- Merging is done by the contributor but is only allowed after all reviewers
    approved. If the merge will affect other people (e.g. by breaking the API),
    make sure to inform them, e.g. via a Mail on an appropriate mailing list.
    After a merge request is merged, the feature branch shall be deleted.

Never push directly to master or other top-level branches. Never force-push to 
any branch that others are using as well. All your development should happen in
branches inside your namespace. To keep the number of branches on the repository
low, make sure to delete branches that are not needed anymore.

The top-level namespace of the repository should only contain a master branch
and maybe a few branches for specific versions. Those branches should be
protected so that direct pushing is not possible (everything has to go through
merge requests).

#### IV.2. Some rules for the contributors:

- To make life easier for the reviewers and to get your changes merged quickly
    (thus reducing the risk of merge conflicts), try to keep merge requests
    rather small. This means that, where possible, a bigger task should be split
    into smaller sub-tasks that can be merged one after another instead of
    putting everything in one huge merge request.
- When synchronising your feature branch with upstream, you may want to prefer
    git rebase over git merge to keep the history of you branch clean. However,
    when there are complicated merge conflicts, it can be much less painful to
    use merge, which is okay in this case.
- On your own branches, you can do whatever you want, i.e. it is usually okay to
    force push there (and even necessary when you rebase). However, when
    applying changes requested by a reviewer, please do not amend or squash them
    into older commits but add them as new commits. This makes it easier for the
    reviewer to see what changed.
- Do not add functional changes and major reformatting in the same commit as
    this makes review of the functional changes very difficult.
- Do add unit tests for new features
- Do create new demos or update existing demos to make it easier how to use
     the updated API

#### IV.3. Some rules for the reviewers:

- To make life easier for the contributors, try to provide reviews in a timely
    manner. The contributor may be blocked in continuing their work while the
    merge request is under review.
- When reviewing, check the following things:
    - Is the style guide followed?
    - Are new features/changes properly documented?
    - And of course: Do the changes look reasonable and correct?
- The one who merges should directly delete the feature branch afterwards.


### V. Documentation

#### V.1 In-code documentation

All code **shall** be documented in-source using Doxygen.
This means that every function, class, struct, global variable, etc,
needs to have a docstring containing some documentation in:
- [Google](https://google.github.io/styleguide/pyguide.html?showone=Comments#Comments)
    format for Python,
- [Doxygen](http://www.doxygen.nl/manual/index.html) format for C/C++.

If the definition and declaration are separated (C/C++), Doxygen should be in
the header, not the cpp file. **Please do not duplicate!**.

If you are using catkin, the package should include the mpi_cmake_modules
package and in the CMakeLists.txt file call the
`add_documentation()` macro.
The documentation can then be build via
`colcon build --cmake-args -DGENERATE_DOCUMENTATION=ON`.

Make the documentation as compact as possible. Avoid boilerplate formulations
that do not add useful information, e.g. instead of
"This function returns foobar" simply write "Returns foobar".

Also add regular comments to the code whenever you feel that they would help a
future reader to more easily understand what that code is doing.

#### V.2 Unit-tests and demos

Please do **NOT** neglect the power of the continuous integration and a nice
written demos in terms of documentation. So **please** take the time to:
- write some unit-tests. See how to write a unit-tests from the tests folder in
    this package.
- write demo executables to make the external user understand how your
    API should be used.