Skip to content

Pull Request Checklist

b-schubert edited this page Sep 10, 2015 · 1 revision

What should you check before you open a pull request

  1. Does it build?
    • execute python setup.py sdist
  2. Do all tests pass?
    • if a test that is unrelated to your changes fails, check the current remote develop branch if the error occurs also there. If this is the case, complain to the one who broke it to fix it.
  3. Is your code documented?
  • If you added new classes or new methods, make sure that they are documented. This includes all public methods, their parameters, and the class itself. While it is not explicitly required by the coding convention, documenting also non-public members and methods is always a good idea.
  1. Does your code introduce changes to the API?
    • If so, make sure that the documentation is up-to-date.
  2. Is your code (regression) tested?
  • All public methods of a classes need to have a corresponding test class in src/tests/Test<class>.py which includes tests for all public methods. And no, "I will add tests in the next pull request is not an option".
  • If you fix a bug, make sure to add a corresponding regression test to the test suite.
  1. Rebase before you open a pull request!
  • You should rebase your branch on develop before you open a pull request to have all recent changes and to ease the merge. If you pushed your branch already to origin before, git will most likely tell you after the rebase that your local branch and the remote branch have diverged. This is expected! If you are sure that the remote branch does not contain any commits you don't have in your local, rebased version, you can safely push using git push -f origin <branch-name> to enforce overwrite. If not, contact your local git expert on how to get the changes into your local branch.
  • Each commit should represent one logical unit. Consolidate multiple commits (squash them) if they belong logically together or split single commits if they are unrelated. For example: do not commit code formatting together with a one-line fix, this makes it very hard later on to figure out what the fix was and which changes were inconsequential). It is ok if your history shows how the code was generated and the history does not need to be perfect but it needs to be useable in the sense that others can go back later and understand the individual steps you took to arrive at your solution.
  • Each pull request should add one feature or fix one bug. If you have multiple features or fixes in a pull request, you might get asked to split your request and open multiple pull requests instead.
  1. Describe what you changed in the pull request!
  • When opening the pull request you should give a detailed description of what you changed and why. In some cases the individual commit messages are enough, but in most cases it is more convenient for the reviewers to have an overview of what has changed and what you intended with the changes. Include a clear rationale for your changes and add benchmark data if available, see this request for an example of how a how a good pull request looks like.
Clone this wiki locally