Pull Request Checklist

Below is the criteria that will be used in the process of reviewing pull requests (PR):

  1. The content of the PR fits within the scope of the project - Scope

  2. The code included in the PR is written in good pythonic fashion, and follows the style of the project - Coding Style

  3. The code directly resolves a previously raised issue - Issues

  4. PR does not cause unit tests and builds to fail

  5. Changes are reflected in documentation - Documentation

CI Testing

We utilize Travis to perform our continuous integration, or CI. Two types of tests are run: unit tests and tests on the example notebooks. The former are used to perform more granular testing over the project, while the latter ensure our example notebooks are runnable.

Unit Tests

Unit tests should cover all public methods and a sufficient set of use cases to reduce the amount of bugs that make it into releases. Unit tests are run using pytest from the project directory:

$ ls
examples/ LICENSE serpentTools/ tests/ ...
$ pytest

If the pytest-cov package is installed, you can view the coverage, or how much of the project is touched by tests, with:

$ pytest --cov=serpentTools

It is always preferable to increase coverage to decreasing coverage, but minor deviations are acceptable. The coverage is not a factor in passing or failing CI, but substantial changes to the test suite should serve a valid purpose if the coverage does decrease.

Notebook Tests

We try to provide a Jupyter notebook for each of the main readers in the package. These are converted to be used in our main documentation, and serve as a handy way for new users to discover the features provided with serpentTools. In order to ensure that these are valid notebooks as changes are introduced, our CI converts these to python scripts and runs them using the test notebooks script. It is beneficial to run this script during major changes to the API, as well as correcting any errors or deprecated/removed features.

Lint

We also recommend using a linter to analyze the project for code that might induce errors and does not conform to our style guide - Coding Style. This is applied on proposed changes using flake8. Code to be merged into this project should not cause any unit tests to break, nor introduce lint. If you are working on a feature/bug-fix branch, you can compare the lint that would be introduced with

$ git diff --unified=0 develop | flake8 --diff

Here, develop is the intended target branch into which these changes will be merged.