Skip to content
This repository has been archived by the owner on Mar 15, 2024. It is now read-only.

Latest commit

 

History

History
208 lines (146 loc) · 7.13 KB

contributing.rst

File metadata and controls

208 lines (146 loc) · 7.13 KB
Raw

Contributing

This document describes how to contribute to librobinhood and details a few rules the project implements.

Overview

Issues should be submitted on GitHub. Questions can be sent to one of RobinHood's mailing lists:

Subscribe to either mailing lists to answer others' questions.

A third, low-traffic, moderated mailing list provides news and annoucements about the project: robinhood-news.

Reviews happen on GerritHub, and so patches must be submitted there too.

Issues

There are no particular guidelines to submit good issues, at least for now. This is meant to evolve as the project matures.

Patches

To submit a patch, follow Gerrit's documentation on the subject. Alternatively, run the commands below:

cd librobinhood/
git remote add gerrithub https://review.gerrithub.io/cea-hpc/librobinhood
git config remote.gerrithub.pushurl https://<login>@review.gerrithub.io/a/cea-hpc/librobinhood
git config remote.gerrithub.push HEAD:refs/for/main

Also note that you may need to log in at least once on GerritHub before you can push changes.

Coding style

Roughly speaking, RobinHood's coding style is a mix of the Linux kernel coding style and Python's PEP 8. Refer to the RobinHood coding style for more information.

Tests

To be accepted upstream, your patch will need positive reviews, and also to pass tests. Those tests will be run (semi-)automatically [1] once you upload your change, but you might want to make sure they pass before pushing.

To do so, you can run the following:

meson builddir
ninja -C builddir/ test

It is generally a good idea to use sanitizers with meson's -Db_sanitize=address,undefined option. It is also recommended to run clang's static analyzer scan-build. Finally, consider checking test coverage:

meson -Db_sanitize=address,undefined -Db_coverage=true builddir
ninja -C builddir test
ninja -C builddir scan-build

Most of these options are well documented on meson's website.

[1]hopefully, the "semi-" part is only temporary

Commit messages

RobinHood uses the following format for commit messages:

[<component>: ]a short description

[A long description that can span multiple lines.

And contain several paragraphs.]

[Change-type: {bugfix, feature}]
[Breaking-change: description of the breakage]
Signed-off-by: Your Name <your.name@example.com>
Change-Id: I0123456789abcdef...

Where:

  • component is the name of the component impacted by the change, if there is one; [2]
  • a short description usually starts with a verb;
  • A long descrip... contains details of the patch (eg. the context, performance measures, references, ...);
  • Change-type indicates the type of change introduced with this commit;
  • Breaking-change indicates that the commit is a breaking change;
  • Signed-off-by is used as a Developer Certificate of Origin;
  • Change-Id is used by Gerrit to track changes across revisions.

The order of the trailers at the end of a commit is unimportant. You can add them manually, or use the git interpret-trailer command.

The Change-type trailer is only required if the commit clearly fits into one of the supported categories (ie. bugfix or feature). [3] It is meant to be parsed programmatically to then generate a changelog.

The description after Breaking-change is also meant to appear in a release's changelog.

You can use git commit's -s/--signoff option to add the Signed-off-by trailer automatically.

Gerrit provides a commit-msg hook to generate the Change-Id trailer. You can fetch it with:

curl -Lo path/to/librobinhood/.git/hooks/ \
    https://review.gerrithub.io/tools/hooks/commit-msg

Refer to the documentation for more information.

Besides those mentionned above, you can add any git trailer you find relevant. Here is a set of trailer tokens commonly used in RobinHood and their meaning:

  • Fixes: #123, the commit fixes issue #123 (it is interpreted by most platforms, like GitHub, and automatically closes an issue); [4]
  • Relates-to: #123, the change is somehow related to issue #123 (platforms like GitHub may render it as a link to that particular issue, which is always nice).
[2]usually it will be the name of a file without its extension, tests, or doc
[3]the list may grow in the future
[4]you may choose to use any other token that is supported by GitHub, although try to stick with fixes

Reviews

Google documents its review practices here. RobinHood hopes to implement them. It makes for an interesting read overall, whether you intend to submit a patch or review one.

Key takeways are:

  • patches do not have to be perfect, they just need to increase the overall quality of the project;
  • make life easy for reviewers;
  • be nice.

Landing

RobinHood patches are systematically reviewed before they are merged.

Authors may negatively score their own patch to prevent it from landing. But they must never positively score their own patch. [5]

To be merged, a patch must:

  • be fast-forwardable, or trivially rebasable;
  • pass tests;
  • not have any -1 or -2;
  • be assigned to at least two (active) reviewers;
  • have at least one +1.

Once these conditions are met if the patch has at least two +1s, it is merged upstream. Otherwise, reviewers are granted 48h (or until the next +1) to oppose to the patch's landing. If they do not, the patch will be merged upstream.

Reviewers can ask to extend the 48h period, in which case the patch will not land until they submit their review or the extension expires.

[5]it only makes it harder for the gatekeeper to find patches ready to land