Contributors Guide
In raven-II we encourage collaborative work.
Everyone is welcome to join and to implement a new feature, fix bugs, give general advice, etc. Also, we try to discuss everything and to review each other's work so that many eyes can see more thus raising the quality.
As some of you already know, software development is not just coding. Many software engineering tasks have to be done in order to produce good code. For example: setting up infrastructure, designing, testing, documenting, assisting new developers (we are doing it here), and of course programming.
But even programming is not all about writing the code, it is about writing the code and preparing it so that the code can be understood and maintained by others and included in the project.
Both producing the code and bringing it to the project are important parts of the game -- without the code there is nothing to bring in, and having the code outside is a no-win for anyone.
As already said above, we review changes. This idea was borrowed from successful projects like Linux, Python, SAGE and many more. In short, each change is first reviewed by other developers and only when it is approved is the code pushed in.
Just like it takes effort to write good and clear code, reviewing other's work needs effort too. There are best practices we try to follow so that reviewing is fun and effective for both the author and the reviewer.
Raven-II pull requests are managed by Applied Dexterity and the BioRobotics Lab
Your first step will be to find an issue to fix or a feature to add. You then make a "pull request" when you have completed and initially tested a modification to a branch of the code (a pull request could also be made from a forked version of the project). The request goes to project maintainers who then review the changes and can make comments in return, and when satisfied can merge the changes into a main or pre-release branch.
The basic work-flow is a follows:
- Create your git environment, if it was not created earlier.
- Pick an issue to fix or a feature to add.
- Create a new branch from the branch which your change is targeting.
- Modify code and/or create tests of it.
- Be sure that all functional and code tests of Raven-II pass. (Functional testing document draft here )
- Only then commit the changes.
- Create a pull request on GitHub.
After you've submitted your pull request, it will be reviewed by reviewers. They may request you to make changes or run further tests, in which case you may need to:
- Update your pull request
- Synchronize with master uw-biorobotics/raven2
All those are described in Workflow process, but before you read that, it would be useful to acquaint yourself with Coding conventions in Raven-II.
By submitting your change (e.g. patch or pull request), you agree to let your work be licensed the. GNU LESSER GENERAL PUBLIC LICENSE v3.0 (see the LICENSE file for details) covers all files in the Raven-II repository unless stated otherwise.
The first step to contributing to the code base is creating your development environment. This is done only once.
Currently using ROS kinetic, we recommend doing full install
Linux-like systems:
follow the instructions on the ROS wiki - http://wiki.ros.org/kinetic/Installation/Ubuntu
A docker container will be coming later this year to facilitate development in these environments. (You can look at the early container with docker pull abrahq/raven_dev)
You can check the biorobotics lab RAVEN II wiki forum for hints or previous work on your issue/feature. Advice is also available from Applied Dexterity and RAVEN developers around the world via email or posting on the wiki forum.
There is a list of open issues in the issues tab. If you find another issue, open a claim on the issue page before addressing it so that people can find it.
The Raven-II is a development platform so this is where we expect to see the most contributions. Features should be new, not block the control loop from running in real time, and help other users of the Raven-II platform.
Do not forget that all new functionality should be tested, and all new methods, functions, and classes should have doctests showing how to use them.
Keep in mind, doctests are not tests. Think of them as examples that happen to be tested. Some key differences:
- write doctests to be informative; write regular tests to check for regressions and corner cases.
- doctests can be changed at any time; regular tests should not be changed.
In particular, we should be able to change or delete any doctest at any time if it makes the docstring better to understand.
Be sure that all tests of raven_2 pass. (A testing document is in the works)
Remember to use clang-format and cpplint_raven.py after making any changes.
The commit message has two parts: a title (first line) and the body. The two are separated by a blank line.
Title (summary)
Commit message titles summarise what the commit does.
Tools like git shortlog or even GitHub only show the first line of the commit by
default, so it is important to convey the most important aspects of the commit in the first line.
-
Keep to 71 characters or less.
This allows the one-line form of the log to display the summary without wrapping.
-
Do not end with a period (full stop).
-
Provide context for the commit if possible,
e.g.
integrals: Improved speed of heurisch()instead of justImproved speed of heurisch()
A commit won't always be seen in the context of your branch, so it is often helpful to give each commit some context. This is not required, though, as it is not hard to look at the commit metadata to see what files were modified or at the commit history to see the nearby related commits.
Try to avoid short commit messages, like "Fix", and commit messages that give no context, like "Found the bug". When in doubt, a longer commit message is probably better than a short one.
Body
Commit messages are intended for human readers, both for people who will be reviewing your code right now, and for people who might come across your commit in the future while researching some change in the code. Thus, include information that helps others understand your commit here, if necessary.
-
Make sure to leave a blank line after the summary
-
Keep all lines to 78 characters or less (so they can be easily be read in terminals which don't automatically wrap lines.)
-
Give an overview of what the commit does if it is difficult to figure out just from looking at the diff.
-
Include other relevant information, e.g.
- Known issues
- A concrete example (for commits that add new features/improve performance etc.)
-
Use bullet lists when suitable
-
Feel free to use Unicode characters, such as output from the Raven-II Unicode pretty printer.
-
Use plain English
Example of a good commit message
Here is an example commit message (from a commit from the sympy project):
integrals: Improved speed of heurisch() and revised tests
Improved speed of anti-derivative candidate expansion and solution
phases using explicit domains and solve_lin_sys(). The upside of
this change is that large integrals (those that generate lots of
monomials) are now computed *much* faster. The downside is that
integrals involving Derivative() don't work anymore. I'm not sure
if they really used to work properly or it was just a coincidence
and/or bad implementation. This needs further investigation.
Example:
In [1]: from sympy.integrals.heurisch import heurisch
In [2]: f = (1 + x + x*exp(x))*(x + log(x) + exp(x) - 1)/(x + log(x) + exp(x))**2/x
In [3]: %time ratsimp(heurisch(f, x))
CPU times: user 7.27 s, sys: 0.04 s, total: 7.31 s
Wall time: 7.32 s
Out[3]:
⎛ 2 x 2⋅x x 2 ⎞
log⎝x + 2⋅x⋅ℯ + 2⋅x⋅log(x) + ℯ + 2⋅ℯ ⋅log(x) + log (x)⎠ 1
──────────────────────────────────────────────────────────── + ───────────────
2 x
x + ℯ + log(x)
Previously it took 450 seconds and 4 GB of RAM to compute.
Be sure that you are in your own branch, and run::
$ git push github issue/11
This will send your local changes to your fork of the Raven-II repository. Then navigate to your repository with the changes you want someone else to pull:
https://github.com/mynick/raven2
Select branch, and press the Pull Request button.
After pressing the Pull Request button, you are presented with a preview page containing
- a textbox for the title
- a textbox for the description, also referred to as the opening paragraph (OP)
- the commits that are included
The title and description may already have been pre-filled but they can be changed. Markdown is supported in the description, so you can embed images or use preformatted text blocks. You can double check that you are committing the right changes by
- switching to the
Commitstab to see which commits are included (sometimes unintended commits can be caught this way) - switching to the
Files Changedtab to review the diff of all changes
When you are ready, press the Send pull request button. The pull request is sent immediately and
you’re taken to the main pull request discussion and review page. Additionally, all repository collaborators and followers will see an event in their dashboard.
If there isn't an issue that the pull request addresses, one should be created so even if the pull request gets closed there is a redundant reference to it in the issues.
You might feel that all your documentation work is done if you have made good commit messages. But a good title and description will help in the review process.
The title should be brief but descriptive.
-
Give your pull request a helpful title that summarises what your contribution does. In some cases
Fix <ISSUE TITLE>is enoughFix #<ISSUE NUMBER>is not enough. Example, don't write "fixes #11" there; such references are more useful in the description section. -
do include the prefix "[WIP]" if you aren't ready to have the pull request merged and remove the prefix when you are ready
The description (also called the OP or Opening Paragraph) is a good place to:
- show what you have done, perhaps comparing output from master with the output after your changes
- refer to the issue that was addressed like "#11"; that format will automatically create a link to the corresponding issue or pull request, e.g. "This is similar to the problem in issue #11...". This format also works in the discussion section of the pull request.
- use phrases like "closes #11" or "fixed #11" (or similar that [follow the auto-close syntax] (https://help.github.com/articles/closing-issues-via-commit-messages) and are also [discussed here] (https://github.com/blog/1506-closing-issues-via-pull-requests)) then those other issues or pull requests will be closed when your pull request is merged. Note: this syntax does not work in the discussion of the pull request.
See also github's own guidelines for pull requests
If you need to make changes to a pull request there is no need to close it. The best way to make a change is to add a new commit in your local repository and simply repeat push command:
$ git commit
$ git push github issue/11
Note that if you do any rebasing or in any way edit your commit history, you will have to add
the -f (force) option to the push command for it to work:
$ git push -f github
You don't need to do this if you merge, which is the recommended way.
Sometimes, you may need to merge your branch with the upstream master. Usually you don't need to do this, but you may need to if
-
Someone tells you that your branch needs to be merged because there are merge conflicts.
-
Github/Travis tells you that your branch could not be merged.
-
You need some change from master that was made after you started your branch.
Note, that after cloning a repository, it has a default remote called origin
that points to the uw-biorobotics/raven2 repository. And your fork remote named as
github. You can observe the remotes names with the help of this command::
$ git remote -v
github git@github.com:mynick/raven2.git (fetch)
github git@github.com:mynick/raven2.git (push)
origin git://github.com/uw-biorobotics/raven2.git (fetch)
origin git://github.com/uw-biorobotics/raven2.git (push)
As an example, consider that we have these commits in the master branch of local git repository::
A---B---C master
Then we have divergent branch issue/11::
A---B---C master
\
a---b issue/11
In the meantime the remote uw-biorobotics/raven2 master repository was updated too::
A---B---C---D origin/master
A---B---C master
\
a---b issue/11
There are basically two ways to get up to date with a changed master: merging and rebasing. In general, rebasing should only be used before you make a pull request to Raven-II. Once the pull request is made your code is considered public and the history should not be changed through rebasing. See the Git page for different techniques to do this
If you prefer to test code manually, you will first have to set up your environment as described in the Workflow process section. Then, you need to obtain the patched files. If you're reviewing a pull request, you should get the requested branch into your Raven-II folder. Go into your folder and execute ("username" being the username of the pull requester and "branchname" being the git branch of the pull request)::
$ git remote add "username" git://github.com/"username"/raven2.git
$ git fetch "username"
$ git checkout -b "branchname" "username"/"branchname"
Then try to execute the code on a Raven-II. If there are any problems, notify the author in the pull request by commenting.
A pull request must meet the following requirements during review before being considered as ready for release.
- All tests must pass.
- Rationale: We need to make sure we're not releasing buggy code.
- If new features are being implemented and/or bug fixes are added, tests should be added for them as well.
- The reviews (at least 1) must all be positive.
- Rationale: We'd like everyone to agree on the merits of the patch.
- If there are conflicting opinions, the reviewers should reach a consensus.