Skip to content

Kedro Framework Pull Request and Review team norms ✍️

Merel Theisen edited this page Nov 17, 2022 · 1 revision

Pull request team norms

  1. Fill out the PR template and add sections where relevant.

  2. Add a clear description to your PR. This is especially important for bug fixes, because it helps to verify if the bug is still fixed in the future. The description should at least mention the following:

    1. Defined scope and context
    2. Explanation on how the implementation solves the issue and what decisions were made to reach the implementation
    3. How the implementation was tested with steps to reproduce if tests were done manually
    4. If the implementation in the PR is different from the initial idea/issue, summarise why this is the case
    5. Link relevant discussions/issues
  3. Create an easy to follow development story through traceable commits.

  4. Request two people to review your code.

  5. Before requesting a review, make sure your PR is complete. It should contain tests, or have been tested manually, be linted and contains docs. If your PR is still in draft it’s okay to be less complete, but you have to be clear what you’d like a draft reviewer to focus on.

  6. Make sure to follow up on feedback/approval within reasonable time and address all comments you get from reviewers, so they know you have seen their feedback and taken it into account.

  7. Break up pull requests if they are very big: as a rule of thumb a PR should focus on one feature/issue. Documentation can be done separately if a PR is already very large.

  8. Try to link your PR to an issue for traceability. The exception to this is very small PRs that fix typos, minor bugs or CI failures.

Review team norms

  1. When you’ve been requested to do a review make sure to do this in a timely manner. Also, make sure to remember to come back to the PR after code suggestions have been addressed. Try not to let PRs get stale.

  2. Read the PR description and try to address any questions or discussion points raised in your review.

  3. Give constructive feedback on implementation, code style, wording/spelling of doc strings and documentation. The aim is to ensure that any merged code is of good quality, consistent with the rest of the code base, and doesn’t contain any bugs.

  4. Always explain why you think something should be changed and give suggestions.

  5. Ask questions if you don’t understand something in the PR. Also remember that there are no stupid questions and PRs are an excellent way to learn things.

  6. The "Request Changes" feature on Github can be used, but when you do, you should explain why you have requested the changes and give clear suggestions.

  7. When changes were tested manually or the PR creator has explicitly asked reviewers to test the code, make sure to check out the code and test it.

  8. PRs are an opportunity to learn for both the creator and reviewers. So take advantage to teach and learn above and beyond immediate actions.

Clone this wiki locally