This is an internal documentation. There is a good chance you’re looking for something else. See Disclaimer.

Git Guidelines

Branch names

You are generally free to choose your own branch names, but some helpful conventions exist.

  • Name feature branches you intend to create Merge Request for like pr/<version>/<feature name>

    • For backend changes, an example would be pr/3.13/my-cool-feature

    • For client changes, an example would be pr/313/my-cool-feature

    • For features intended for master, simply use pr/<feature name>

    • When a branch is named like this, created MRs will automatically have the correct target branch selected

  • A few other branch name prefixes exist

    • feature/<feature name> for long-lived feature development

    • users/<user name>/<branch name> for personal branches that will usually never get merged

    • poc/<feature name> for experimental development that will not get merged from this branch directly

    • forks/<installation name> for special customer branches that get deployed

      • keep in mind, these generally need additional setup in CI and CD

Code Reviews

  • Every changeset has to be reviewed by another developer (via GitLab). No reviewer is required if:

    • Only text resources are adjusted

    • An error in a form is fixed (e.g. order of fields is changed)

    • A layout issue in a report is fixed

    • An auto-merge conflict is fixed and no logic is manually adjusted (e.g. only imports are fixed or ImmutableList is replaced by List)

  • If non-customer code is changed, this part of the change must be reviewed by a DEV

  • When doing code reviews be respectful and remember also to praise good code.

  • Be clear and add suggestions.

  • Use conventional comments for being precise and clear in your intent.

  • The author has the responsibility to merge a pull / merge request.

  • The reviewer should (if possible) look at the review code within 24 hours and give feedback or approve the change.

  • It is not (necessarily) part of the reviewer’s job

    • to ensure that the change works exactly as described in the ticket.

    • to start the application and test it manually.

    • that generated code snippets (like changesets) are completely correct.

  • If the reviewer is familiar with the code and/or feature, they may also provide comments on how to improve the implementation.

Commit Message

Commit message conventions differ for different repositories. See Backend and Frontend for our two main repositories.

Conventional comments

Used for structured review comments.

Conventional Comments documentation

<label> [decorations]: <subject>

[discussion]

  • label - This is a single label that signifies what kind of comment is being left.

  • subject - This is the main message of the comment.

  • decorations (optional) - These are extra decorating labels for the comment. They are surrounded by parentheses and comma-separated.

  • discussion (optional) - This contains supporting statements, context, reasoning, and anything else to help communicate the “why” and “next steps” for resolving the comment.

We use the following set of labels:

Label

Description

Blocking / Non Blocking

praise:

Highlights something positive.

Non Blocking

suggestion:

Suggestions propose improvements. Be explicit and clear on what is being suggested and why!

both (use decorations)

question:

Questions can be used to ask the author for clarification or investigation of a specific topic.

both (use decorations)

todo:

TODO’s are small, trivial, but necessary changes.

Blocking

typo:

Similar to todo: where main issue is a mispelling.

both (use decorations)

nitpick:

Nitpick is a trivial preference-based suggestion.

Non Blocking

We use the following set of decorations:

Decorations

Description

(non-blocking)

The comment does not block the code to be approved.

(blocking)

The comment requests a mandatory change to the code to get an approval.

(if-minor)

The comment can be done when the changes are small and do not take a lot of work or risk.

Examples:

typo: mispelled variable name, `isVaild` => `isValid`

suggestion (blocking): Reuse existing helper functions

We have already good tested helper functions for this date calcuation (see `modules/date`)

suggestion (non-blocking,if-minor): Can we refactor this class as well?