Skip to content
This repository has been 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 a pull request

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 a pull request

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