Gatekeeper Guidelines#

Section Author
Ralf Mueller
for gatekeepers,only

The gatekeeper’s task is to take the final decision about whether code enters the master/main branch, and to take care of the merge process. Their decision is based upon the reviewer feedback, the testing results, as well as their personal judgement on the change set. If needed, gatekeepers can request additional reviews and/or raise issues by themselves that must be solved by the contributors.

Main Requirements#

  1. The MR has been approved by the reviewer(s)

  2. Review(s) have covered the full change set

  3. Full suite of tests has been performed and passed

  4. Feature branch is reasonably up-to-date wrt. the target branch

If any of the above points is not given, set the MR back to ‘Draft’, assign to the contributor and ask for resolution of the issue(s).

In more detail#

Final Review#

Check for decent reviews on all added changes. Depending on the character of the changes multiple reviews can be needed. Request more if needed. See reviewing guidelines for more.

Testing#

Since contributors are resposible for rebasing and testing, the following steps are optional

  1. The last commit of a merge request (MR) must undergo the full testing. There are exceptions: MRs that

    • only update the RELEASE_NOTES.md

    • only change documentation

    • only change test, pre- or postprocessing scripts, not used in CI pipelines or buildbot tests

    • only update tests of specific builders

  2. What full testing means, can depend on the contributor group, see development workflow for more.

  3. All GitLab CI jobs that show up in a merge request pipeline are supposed to succeed.

Formal Aspects#

  1. The title of the merge request must give a general idea of the changes at least for those who are involved in the ICON development. It will be part of the commit message and hence part of the release notes of the next public release. If done wrong, it creates additional work during release process.

  2. The link to the MR will be part of the commit message. If there is need to provide additional information to that, please keep it short and “recommendations” (requirements) provided in 3. Any links in the short description are discouraged (even those that reference issues and merge requests).

  3. As a sanity check go through commits and changes of the MR and ensure all of them belong there and not some merge/rebase artifacts. If there are problems, tell the author to address them.

Merge#

  1. Press Merge for the most simple commit message or

  2. Edit in case more information is really needed

Additional things to consider#

  1. The title of the merge request should be a single sentence, start with a capital letter, do not have commas, semicolons or dots (dashes and colons often make sense though) and, be no longer than 80 characters (ideally, no longer than the generally recommended 72 characters minus the length of the fork-specific prefix [mpim] , i.e. 72 - 7 = 65 characters). Ideally, the title starts with a verb in the imperative mood.

  2. (optional) Check/Edit the commit message before click the Merge or Set to auto-merge buttons:

    1. The first line should be the title with a fork-specific prefix (e.g. [mpim] )

    2. The second line must be empty

    3. The third line should be full URL to the MR in question

    4. The fourth line must be empty

    5. If there should be a short description (see 5. above), the fifth line must be the first line of the short description (the ### Short description line or any other template artifacts must be removed). All other parts of the description must be removed (together with the template artifacts). It is possible that the title is enough to describe the changes and the short decription is redundant but the author decided to provide some extra information. That information must be treated as detailed description and removed from the commit message. The short description must be followed by a single empty line.

    6. The next lines (e.g. Co-authored-by, Merged-by, etc.) are automatically generated by GitLab and must be kept. However, the users often fail to configure their git properly. You must double-check the Co-authored-by list:

      1. The names must be correct (no nicknames or abbriviation). In case of doubt, ask the authors if they’d submit a paper to Nature under the names you see on the list.

      2. The emails must be valid or an automatically-generated private commit email by GitLab. Other ways to hide real emails (e.g. 64255981+name@users.noreply.github.com) are discouraged but tolerable. Either way, the email must be registered in the GitLab user profile of the author. We can’t check this in advance, therefore, in case of doubt, we ask the author if they did so. There will be a technical solution in GitLab.

      3. Sometimes it is easier for the author to list the co-authors in the detailed description than getting them configure their git and clean up the history. Do not forget to add those on the list the way GitLab would do it (i.e. with the Co-authored-by: prefix and so on).

      4. Remove the duplicates (e.g. same name and different emails).

Happy Merging

On this page