Skip to content

Instantly share code, notes, and snippets.

@harssh
Forked from katyhuff/pr-checklist.md
Created February 13, 2023 15:40
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save harssh/6db8963aa8cabd9e53a40002ced1fd70 to your computer and use it in GitHub Desktop.
Save harssh/6db8963aa8cabd9e53a40002ced1fd70 to your computer and use it in GitHub Desktop.
Pull Request Review Checklist
  • Read the PR description
  • Read through all the changes, considering the following questions.
    • Is there anything you can praise about this PR? Start with praise.
    • Are variable names brief but descriptive?
    • Are new/changed functions no longer than a paragraph?
    • Do all function parameters have default values where appropriate?
    • Is the code clear and clean? (see Robert C. Martin's Clean Code)
    • Is there enough documentation?
    • Does the programming style meet the requirements of the repository (PEP8 for python, google for c++, etc.)
    • If a new feature has been added, or a bug fixed, has a test been added to confirm good behavior?
    • Does the test actually test the new/changed functionality?
    • Does the test successfully test edge cases?
    • Does the test successfully test corner cases?
    • If the repository has continuous integration: does the PR pass the tests?
    • If there is no continous integration, check out the branch locally, build, and run the tests.
    • Do the tests pass on your machine?
    • Is the PR free of random cruft (built files, swp files, etc.)?
    • Do all files in the PR belong in the repository?
    • If the PR deletes files, is this appropriate?
    • If the PR adds files or data, are these new files compatible with the repository license?
    • Does this PR close an issue? If so, be sure to descriptively close this issue when the PR is merged.
    • Make a review, leaving kind comments and suggesting changes where needed (to resolve the above).
  • When you approve of the PR, merge and close it.
  • Thank the author for their contribution.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment