Contributing

Bruno Bottazzini edited this page Jun 10, 2016 · 35 revisions

If you're going to contribute with code just keep reading this but if you're going to contribute to our wiki please take a look at our Contributing to the wiki page.

Before you contribute

We don't ask for any kind of copyright assignment, you are owner of the copyright of your changes. However we need to ensure you're the author of your changes.

So please indicate that you certify you are able to contribute the code by adding a signed-off-by line at the end of your commit message, using your real name.

This indicates that your contribution abides to the Developer Certificate of Origin rules.

When commiting you can use the -s flag to automatically append the "sign-off-by" line in your commit message:

git commit -s

Alternatively you can add the following at the bottom of your commit message:

Signed-off-by: Random Developer Full Name <random@developer.example.org>

Contributions flow

To make contributions to Soletta™ Framework, you should:

  1. Fork the repository.
  2. Make commits in series as short as possible - avoid adding a lot of unrelated patches in the same pull request.
  3. Make a pull request for the selected branch. The pull request subject should be short and descriptive, issues linked to it should be in its request message, not in the commit messages.
  4. Wait a few days for reviews - if after a couple of days you don't see any comments, what is no expected to happen, please poke us by email / IRC.
  5. You need at least one of the core developers to agree that your code is good to be merged (+1), but depending on the pull request complexity, wait for more than one acknowledgment.
  6. If no changes to your proposed code are required, the pull request will be rebased on top of the master branch, pushed and closed by one of the core developers.
  7. If changes are required:
  • Make a new pull request with an updated version on its subject, e.g "Add GPIO support for RIOT [v2]"

  • Link the previous pull request to this one (by GitHub's citation markup), so people can see previous discussions

  • Add a changelog between pull request versions in the pull request messages

  • Close previous pull requests when uploading new ones, to avoid people commenting in outdated work

  • Go back to step 4

    1. When the PR is good to be integrated, there are two possibilities:
    2. the author is a Soletta commiter
    3. the author does not have Soletta commit rights

    If you're in case 2, just wait until a commiter integrates your patch. If you're in case 1 and will merge a PR, you you must never, ever, hit GitHub's merge button to achieve that. Instead, rebase the PR onto current master branch, then push the changes directly to the master branch (via command line). The master branch is so that it only accepts fast-forwards, so it complains if you did not rebase the PR before. The commands for commiter to integrate a PR will then be as following:

    git checkout reviewed_branch
    git fetch origin
    git rebase origin/master
    git push origin HEAD:master
    
    1. Close the PR after it was integrated and pushed, commenting it was done in the PR's web page. Alternatively you may use "Closes gh-" on the top commit message, so that GitHub will automatically close the PR when it lands upstream.

Commits

Commits should be split by fixed or featured added. Following that rule, try to keep commit series as granular as possible, so if some of them are wrong they can be dropped and others will be applied. Also, it makes it easier to find eventual regressions later. Code review is much easier in this case as well.

Avoid coding styles changes in the same commits of real changes. Also only send coding style change commits if it seems really necessary -- we control code style quality by helper tools.

Please invest some time writing useful commit messages. The following template should be used

component: short description

more details paragraph 1

more details paragraph 2

Signed-off-by: Random Developer Full Name <random@developer.example.org>

Example:

common: add macro for static checking array size

In C there's a feature to improve the check of the array size being
passed to a function, by specifying 'static' before the array
size. However this feature is not supported on C++.

This patch wraps the usage into a macro to let the public headers to be
used in C++ programs.

Signed-off-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>

If it's a bug fix, please add enough information to make it possible to be reproduced.

Logs or backtraces are welcome.

For memory issues, a valgrind log could be pasted as well.

Updates on submodules

Soletta submodules, whenever they need to be moved forward (or changed anyhow) from the point they were at a given time, have to have that signaled to downstream users by means of a special flow. Moving them forward can be achieved with the following command, for example:

git submodule update --remote

After the desired changes, git diff should show you something like this:

diff --git a/src/thirdparty/tinycbor b/src/thirdparty/tinycbor
index f0791a2..b80318a 160000
--- a/src/thirdparty/tinycbor
+++ b/src/thirdparty/tinycbor
@@ -1 +1 @@
-Subproject commit f0791a2a12599c82e9b65f2923eb1cdd6c141e5d
+Subproject commit b80318ab0640efa98147836380c7937a59dc327d

Get those changes in a commit and submit them.

API Conventions

When adding new API to Soletta, please follow our API design conventions.

Coding Style

We follow a style defined in a uncrustify schema. It's all specified in there, but in summary it is similar to Linux's style, but using 4 spaces and no tabs. One should check the style of commits or staging area changes by using our tools/check-style.py. See Git Hooks below.

License

Files should contain a license header, specifying who owns the copyright. Since Soletta is to be used in small operating systems (besides Linux) and these often are compiled as a single statically linked binary we require the Apache 2.0 license.

Whenever adding new files, ensure the license is included at the top of the file. One can check the licensing status of new files from commits using our tools/check-license.sh. See Git Hooks below.

Exceptions are always painful to manage, but if there is an exception to your contribution please let us know and we can discuss about it. Eventually we can accept it in the main repository or create another repository with modules using different licenses. In those cases we need to add entry-points so they are used (many are already supported, such as the flow and linux-micro modules).

Significant changes should also list their author in AUTHORS.

Test and Validate

Before sending patches please run our tests (if you add any sample Soletta application into the patches, the last rule will exercise that):

    make check
    make check-fbp
    make check-fbp-bin
    make samples

If you did changes in C, please check your memory usage with Valgrind (to catch memory errors such as leaks and invalid memory access) and the overall coverage of your tests:

    make check-fbp-valgrind
    make coverage

By taking these steps, you're most likely to get the CI tool's approval on your pull request. Without the bot being green, it's impossible for anyone to accept your patch.

Git Hooks

We recommend contributors set a pre-commit Git hook to check before sending patches:

#!/bin/sh
./tools/check-style.py || exit 1
./tools/check-license.sh || exit 1
exit 0

Save this as .git/hooks/pre-commit and remember to make it executable (chmod +x .git/hooks/pre-commit).

Notice that check-style.py requires uncrustify 0.60 or newer, check this page for more info on how to install it.