Skip to content

Pull Request Guidelines

Rémi Attab edited this page Aug 26, 2013 · 10 revisions

This document will give a few guidelines which should improve the chances that pull requests will be promptly reviewed and merged into RTBKit.

Limit your changes

A pull request should contain a single change. This makes it much easier to review the pull request because it only does one and that one thing can be evaluated independently of any other changes.

A good rule of thumb is to create a seperate branch in git whenever you make changes that you want to merge upstream. This does require that you're comfortable with manipulating branches in git.

Document your changes

Your primary goal when documenting a pull request is to convince the reviewer that your changes are needed and significantly improves RTBKit. A good description will contain the following elements:

  • A description of the problem. This is particularly important because it will determine how critical your pull request is.
  • An overview of the propossed solution.
  • Why this solution was chosen over other alternatives.
  • A description of executed tests that supports your solution.

While all these elements should be present, the size of each will heavily depend on the size of the proposed changes. A few lines is enough for a minor bug fix but a major architectural change will probably require several paragraphs.

Discuss the changes

If you see a glaring flaw within RTBKit, resist the urge to jump into the code and single-handedly make major architectural changes. I know it can be tempting but it's a good idea to first discuss the proposed changes on the mailing list. It may turn out that someone is already working on this fix or maybe there's a good reason why that flaw exists. Even better, you may get some feedback which will improve your final solution and the code review will probably be far more streamlined if the reviewer is already familliar with the solution.

Again this is all relative the size and scope of your changes. A good rule of thumb is to estimate how much time would be wasted if the pull request was rejected. If it's a couple of hours then you can probably dive head first and eat the loss. Otherwise, making a quick check on the mailing could save you lots of time down the line.

Follow up

While not ideal, pull requests do occasionally get forgotten. When this happens, a gentle reminder is usually needed to move the process along.

Clone this wiki locally