Skip to content
hyperthunk edited this page Dec 10, 2012 · 3 revisions

Contributor guide

Community contributions are most welcome! These should be submitted via Github's pull request mechanism. Following the guidelines described here will ensure that pull requests require a minimum of effort to deal with, hopefully allowing the maintainer who is merging your changes to focus on the substance of the patch rather than stylistic concerns and/or handling merge conflicts.

This document is quite long, but don't let that put you off! Most of the things we're saying here are common sense and if you strongly disagree with some, we're open to discussing that too! :)

With this in mind, please try to observe the following guidelines when submitting patches.

1. Check to see if your patch is likely to be accepted

To avoid wasting time making changes, only for maintainers to reject your pull request on conceptual grounds, it is best to submit an issue on the github issue tracker in the first instance. This should engender a discussion about the proposed feature (or bugfix), allowing everyone undertake significant work only when there's confidence the resulting patch is likely to be accepted.

2. Make sure your patch merges cleanly

Working through pull requests is time consuming and this project is entirely staffed by volunteers. Please make sure your pull requests merge cleanly whenever possible, so as to avoid wasting everyone's time.

The best way to achieve this is to fork the main repository on github, and then make your changes in a local branch. Before submitting your pull request, fetch and rebase any changes to the upstream source branch and merge these into your local branch. For example:

## on your local repository, create a branch to work in

$ git checkout -b bugfix-timer-deadlocks

## make, add and commit your changes

$ git checkout master
$ git remote add upstream git://github.com/haskell-distributed/distributed-process-platform.git
$ git fetch upstream
$ git rebase upstream/master

## and now rebase (or merge) against your work

$ git checkout bugfix-timer-deadlocks
$ git merge master

## make sure you resolve any merge conflicts
## and commit before sending a pull request!

3. Follow the patch submission rules of thumb

These are pretty simple and are mostly cribbed from the GHC wiki's git working conventions page here.

  1. try to make small patches - the bigger they are, the longer the pull request QA process will take
  2. strictly separate all changes that affect functionality from those that just affect code layout, indentation, whitespace, filenames etc
  3. always include the issue number (of the form fixes #N) in the final commit message for the patch - pull requests without an issue are unlikely to have been discussed (see above)
  4. use Unix conventions for line endings. If you are on Windows, ensure that git handles line-endings sanely by running git config --global core.autocrlf false
  5. make sure you have setup git to use the correct name and email for your commits - see the github help guide

4. Make sure all the tests pass

If there are any failing tests then your pull request will not be merged. Please don't rely on the maintainers to deal with these, unless of course the tests are failing already on the branch you've diverged from. In that case, please submit an issue so that we can fix the failing tests asap!

The full test suite can be run with make test-all. Please look at the testing wiki page for details of all the testing requirements. If we do not support running the full test suite on your platform and there are test failures, this could significantly slow down the process of getting your patch merged. Please note that performance regressions may also prevent patches from being accepted under some circumstances. The default test-all rule runs the performance benchmarks so they can be compared with the latest release easily.

5. Try to eliminate compiler warnings

Sometimes compiler warnings cannot be avoided, but more often than not they are telling you something useful and you should strive to silence them if at all possible. Pull requests that contain multitudinous compiler warnings will take much longer to QA.

6. Always branch from the right place

This project is using a variant of the git-flow branching model. Please be aware of whether or not your changes are actually a bugfix or a new feature, and branch from the right place accordingly. For the attention deficit among us, the general rule is:

  • new features must branch off development
  • bug fixes must branch off master (which is the stable, production branch)

If you branch from the wrong place then you will be asked to rework your changes so try to get this right in the first place. If you're unsure whether a patch should be considered a feature or a bug-fix then discuss this when opening a new github issue.

General Style

A lot of this is taken from the GHC Coding Style entry here. In particular, please follow all the advice on that wiki page when it comes to including comments in your code.

Do not introduce trailing whitespace. Just don't do it. Please. :)

Please avoid including more than one or two blank lines between definitions. Most code has just one, and this is usually sufficient.

Please wrap lines at <= 80 columns - that might seem antiquated to you, but some of us (i.e., me) do things like github pull-request code reviews on our mobile devices on the way to work, and long lines make this horrendously difficult. Besides which, some of us are also emacs users. ;)

Please use spaces and not tabs. If a pull request includes tabs, it will be rejected until they've been cleaned up and converted to spaces. All good editors should manage this sort of thing for you automatically.

Indentation is usually 2 spaces, with 4 spaces used in some places. We're pretty chilled about this, but try to remain consistent. And on that note....

As a general rule, stick to the same coding style as is already used in the file you're editing. It is much better to write code that is transparent than to write code that is short. Please don't assume everyone is a minimalist - self explanatory code is much better in the long term than pithy one-liners. Having said that, we do like reusing abstractions where doing so adds to the clarity of the code as well as minimising repetitious boilerplate.

When it comes to alignment, there's probably a mix of things in the code base right now. Personally, I tend not to align import statements as these change quite frequently and it is pain keeping the indentation consistent. Do however, wrap long import lists as you would a module definition:

-- this is not so good
import qualified Foo.Bar.Baz as Bz
import           Data.Binary       (Binary (..), getWord8,
                                    putWord8)
import           Data.Blah
import           Data.Boom         (Boomable) -- this indentation will be *useless* if we import something from a great big long (package) name

-- this is better
import qualified Foo.Bar.Baz as Bz
import Data.Binary
  ( Binary (..),
  , getWord8
  , putWord8
  )
import Data.Blah
import Data.Boom (Typeable)

Personally I don't care that much about alignment for other things, but as always, try to follow the convention in the file you're editing and don't change things just for the sake of it.

We do not object to using literate haskell sources, but please only do this if you really do need an essay around your code in order for it to be widely understood. Don't just do this for the sake of it, but if you've invented some sparkly new distributed algorithm that bares talking about that much (and have graciously decided to contribute it to the CH platform library), then that's probably fair enough.

Other rules to be aware of

Please DO NOT

  • add dependencies on third party libraries before discussing them with a maintainer first
  • add dependencies to experimental GHC extensions to the Haskell language without discussing them with a maintainer first
  • disable tests because they're failing on your branch
  • rewrite or refactor huge swathes of other people's code without discussing this with a maintainer first

Please DO

  • get involved - these guidelines are not the 10 commandments (or whatever) and are primarily intended to make it easier to contribute!
  • put your point across if you feel there's something we could be doing better
  • go ahead and make changes that have a positive impact on performance - but do discuss the refactoring with a maintainer first (see above)

If you need/want support for a version of GHC that isn't currently supported, please let us know by submitting an issue. We'd like to make sure we have the best possible coverage of GHC releases too!

Please note that we're not against refactoring, especially where doing so would improve the clarity of the code, or the runtime performance of the library itself. We just want to make sure that this kind of work is dealt with in a coherent and structured way that makes everyone's life as easy as possible.