Submitting patches

proxyles edited this page Oct 5, 2011 · 40 revisions

Introduction

The recommended way of sending patches is described on this page. Basically, you push your changes to a git repository and send us an email with a reference to your public git repository and branch.

If you follow the instructions, you should receive a response within one workday.

By following the instructions, you help the Erlang/OTP team to be more productive, ultimately benefiting all Erlang/OTP users.

What will happen with patches sent as attachments?

If you submit a patch in a way other than what is described here, it implies more work for the Erlang/OTP team. In that case we cannot guarantee anything. It is up to the maintainer for that part of OTP to decide whether (s)he will pick up the patch.

If the patch corrects a bug considered severe by the Erlang/OTP team we will take care of it regardless of how it was submitted. This includes bugs related to system startup, corrupt data and bugs causing VM crashes. We never apply patches without verification, and may implement the patch in a way we find better than the original patch.

What about pull requests?

We do not currently use pull requests.
Patches by email is preferred, as the patch discussion will be archived, enabling users to learn from the discussion.

Setting up user information

Please have git setup with consistent user information before sending a patch. Preferably with your real name and a
working email address (the one you use on the erlang-patches mailing list, for example).
For quick reference, here are the relevant git commands:

git config --global user.name "Your Name Comes Here"
git config --global user.email you@yourdomain.example.com

Branching out

If you are submitting a bugfix, or minor improvement:
Create a new branch starting from maint (the maintenance branch for the current major release).

If you are submitting a major change, or a new feature:
Create a new branch starting from master (the development branch for the next major release).

You can do this via the following instructions (assuming you already have a github account and you are signed in):

  • Browse to the Erlang/OTP git project page and hit the ‘fork’ button to create your own fork of the repository (you only have to do this once).
  • After the ‘hardcore forking action’ has completed, checkout a copy of your new fork via ‘your’ clone URL:
    git clone git@github.com:your_username/otp.git
  • cd into the newly created ‘otp’ directory
  • Create a remote pointing to the OTP repository.
    git remote add upstream git://github.com/erlang/otp.git
  • Create a new remote branch to hold your changes (use a descriptive name for the particular change you’re making):
    git push origin origin:refs/heads/new_feature_name
  • Start tracking the new branch (this will switch you to the new branch too):
    git checkout --track -b new_feature_name origin/new_feature_name
  • Pull in all the changes from the master branch on the otp remote into your new branch:
    git pull upstream master
  • Make your changes, commit them according to the guidelines and then send the OTP team a fetch request.
  • If it take you a while to finish your changes or you need some update from a newer master than you branched from, rebase your branch on master. After checking out your branch:
    git fetch upstream
    git rebase upstream/master
    … And fix any merge conflicts. Do not merge from master since we want a straight chain of commits based on a fairly recent master.
  • If you create a patch based on someone else’s you can do exactly that: branch from someone else’s branch (that branches from a fairly recent master).

Each branch should contain conceptually related commits (for example the implementation of a single feature), not a mixed set of random changes.

See forking tips on GitHub, for managing the OTP team branches as the upstreams.

Committing the patch

  • Make separate commits for separate changes. If you cannot describe what the commit does in one sentence, it is probably a mix of changes and should be separated into several commits.
  • To make it possible to use the powerful git bisect command, make sure that each commit can be compiled and that it works.
  • Do not commit commented-out code or files that are no longer needed.
  • Check for unnecessary whitespace before committing with git diff --check.

Making a patch OTP-compliant

The following steps are not necessary if you only want feedback on a proposed change, but are ultimately necessary if you want it to be included in OTP.

  • Make sure existing test cases does not fail.
  • Make sure the patch builds and works on all major platforms (many patches we receive works on Linux but not on Windows).
  • Make sure your changes follow the coding and indentation style of the code surrounding your changes. (At some point in the future, there will be a style guide, either included in the documentation or on some web site, but it is not ready yet.)
  • In most code (Erlang and C), indentation is 4 steps using tabs and spaces. Tabs are always 8 steps. (If you use Emacs, use Erlang-mode, and add (setq c-basic-offset 4) to .emacs to get C code correctly indented.)

Base your branch on maint

  • If you are fixing a bug, write a test case before fixing the bug (so that you’ll know that the test case catches the bug). For applications without a test suite in the git repository, it would be appreciated if you provide a small code sample in the commit message or email a module that will provoke the failure.

Base your branch on master

  • If you are implementing a new feature, also write a new test case and/or test_SUITES. (Note: The primary reason for writing test cases is not to prove that the new feature works correctly, but to make sure that it will be noticed if future changes — perhaps to code that seems unrelated — break the feature.)
  • If you are implementing a new feature, also update the documentation to describe the feature.
  • Make sure the patch does not break backward compatibility. In general, we only break backward compatibility in major releases and only for a very good reason and usually after first deprecating the feature one or two releases beforehand.
  • In general, language changes/extensions or major updates to Kernel and Stdlib also require an EEP (Erlang Enhancement Proposal) to be written and approved before they can be included in OTP.

Sending the patch

Send an email to erlang-patches@erlang.org with a reference to your github repository and name of the branch to pull from. To make it really easy for us to fetch the patch, please include the entire git command. For example:

git fetch git://github.com/bjorng/otp.git my-cool-updates

my-cool-updates is the name of the branch.

Additionally include the following two links for viewing the changes:

https://github.com/bjorng/otp/compare/my-cool-updates
https://github.com/bjorng/otp/compare/my-cool-updates.patch

For the diff to be correct you need to make sure that the default branch configured on github is set to maint or master (which is the case when you fork) and is up to date (same as upstream/BRANCH).

(We also accept inline patches compatible with format generated by git format-patch, but please make sure your email client has not garbled the message.)

Receiving your patch

When we receive your patch, we will do one of the following (typically within a workday):

  • If there are simple things that will need to be fixed (for example, providing more information in the commit message instead of in the email), we will ask you to do that.
  • Otherwise, if your patch is not obviously wrong or inappropriate, we will merge it to the appropriate pu (“proposed updates”) branch, even if it lacks test cases and/or documentation (i.e. it need not be OTP-compliant).
  • Otherwise, if it is obviously wrong or inappropriate, we will tell you so. Reasons for immediate rejections include (but are not limited to):
    • Blatantly breaking backward compatibility.
    • Obviously unsafe coding practices or highly non-portable code.
    • Mixing of many different changes and/or unnecessary re-indentation of code that is not changed. We will ask you to separate the changes into separate commits and/or branches and not change indentation of unchanged code.

You should not base any branches on a pu branch, as it will frequently be rebuilt from scratch on top of the current development branch.

Currently in the ‘pu’ branch

Testing

We run automatic tests and builds of the pu branch.

If a branch included in pu causes a build problem, that branch will be dropped. We will reinstate the branch if/when we receive a correction.

We might also drop a branch if it causes test cases to fail.

We might also drop a branch if we want to address the issue in a different way.

Cooking

Patches will “cook” in the pu branch until they graduate or are dropped.

The following may happen to a patch (one or more times):

  • If a patch is not OTP-compliant, it can be replaced with a new or extended version, either by the original author or by someone else.
  • If build problems are found on some platform (or there are many failing test cases) not found before it was included in pu, it may be removed from pu until the problems have been corrected.
  • Anyone can criticise, suggest improvements, or report that a patch breaks existing applications. If the serious problems remain, and no good way of correcting them can be found, the patch may be dropped.
  • Patches with known issues that have been inactive for a long time will be dropped.

Graduating

Usually a patch will graduate to the maint or master branch when it passes the criteria for being OTP-compliant. Application-maintainer in question will decide when a patch will graduate. For larger patches and for patches on the borderline of breaking backward compatibility, several people in the OTP group will be involved in the graduation process.