Skip to content

Pull Request Guideline

Craig Yu edited this page Mar 5, 2024 · 1 revision

Good pull request review ensures code quality, maintains project consistency, fosters collaboration among team members, and reduces vulnerability by allowing developers to identify and address potential security flaws before they are merged into the codebase.

PR Creation

A good pull request begin at its creation. We shall follow the trinity of PR creation as outlined by GitHub docs. Here is a recap.

  1. Write small PRs: Aim to create small, focused pull requests that fulfill a single purpose. Smaller pull requests are easier and faster to review and merge, leave less room to introduce bugs, and provide a clearer history of changes. In cases where a small PR isn't possible, well-written review guidance as described in step 3 should be followed.

  2. Review your own pull request first: Review, build, and test your own pull request before submitting it. This will allow you to catch errors or typos that you may have missed, before others start reviewing.

  3. Provide context and guidance: Write clear titles and descriptions for your pull requests so that reviewers can quickly understand what the pull request does. In the pull request body, include:

    • the purpose of the pull request
    • an overview of what changed
    • links to any additional context such as tracking issues or previous conversations

    To help reviewers, share the type of feedback you need. For example, do you need a quick look or a deeper critique? If your pull request consists of changes to multiple files, provide guidance to reviewers about the order in which to review the files. Recommend where to start and how to proceed with the review.

PR Review

Reviewing a PR can be time-consuming, and each PR is different, potentially requiring reviewers to pay attention to specific areas. In general, use these guidelines to aid your reviews:

  1. Focus on Code Quality: Prioritize reviewing the code for readability, maintainability, and adherence to best practices.
  2. Security Review: Pay special attention to potential security vulnerabilities and ensure that proper security measures are implemented.
  3. Provide Constructive Feedback: Offer specific, actionable feedback to the author to help them improve their code and skills.
  4. Communicate Effectively: Foster open communication and collaboration between reviewers and authors to address any concerns or questions.
  5. Test Cases: Verify that the code changes include relevant test cases to cover new functionality and edge cases.

PR Merge

The number of approvals required from reviewers depends on the size of the team. In the case of Team Evergreen, 2 approvals are needed by default. However, in cases where this isn't possible, such as when developer(s) are on vacation, 1 approval might be sufficient, but it is recommended to invite developers from another team to conduct a review.

Clone this wiki locally