Skip to content
This repository was archived by the owner on Nov 14, 2022. It is now read-only.

Pull requests

Samuel edited this page Apr 12, 2022 · 9 revisions

Creating Pull requests

Before Making a Pull Request

It is useful to check that the following points are met (if applicable):

  • Only changes related to issue have been made - additional changes should be contained within separate issues to ensure changes are as focused and relevant to the associated issue as possible.
  • The README.md file has been updated to reflect changes made if usage/underlying functionality has been altered.
  • You have performed pylint checks locally. This will ensure your changes do not raise pylint warnings/errors and amendments to python styling can be made before turning into a PR. This keeps the PR commit history as short as possible.
  • All unit tests pass

Content

The pull request should contain:

  • A summary of the work completed
  • A specification of how to validate that the pull request resolves the issue.
    • If the issue contains this then link the issue instead
  • Fixes #issue-number so that the pull request is linked to the issue
  • Any other comments such as I'm not sure this is the best way of doing X, Y, and Z can someone advise?

Project

The pull request should be added to the relevant project associated with the issue. This will mean that Pull request should belong to exactly 2 projects.

Labels

Pull requests should have the same labels as their associated issue with the exception of the status labels (these may differ between the issue and pull request).

Reviewing Pull requests

When should I review a pull request

  • Once a pull request has been opened and the build servers are happy (e.g. the checks have come back green) a pull request should be reviewed.
  • To do this assign yourself as the reviewer
  • Try to review a pull request for every one that you open

GitHub Actions and Static analysis

  • When a pull request is submitted, GitHub Actions will automatically run the unit tests and static analysis on the fully code base.
  • This process takes about 5 minutes and the reports will be added to the pull request.
  • All tests must pass in order to consider merging the code.

How to review

  • Review the code for quality.
    • Check that everything is syntactically correct
    • Ask questions if you are not sure what something does or why it is there
  • Try to the test the pull request using the instructions given by the developer.
    • If there are no instructions or the instruction do not work report this to the developer
  • Try to test around the issue by attempting to break the changes in some way
    • If you do find any problems then report them to the developer
  • Ensure the changes are covered by tests if appropriate

Clone this wiki locally