The #1 Collaboration Tool
Making a Pull Request
A Pull Request (PR) merges the changes in a branch of a repository to the main branch. This means the main branch has all of the updates that need to get approved before they’re implemented. For the basics on how to create a PR, GitHub has a guide available here.
Most of the pull requests done in the ARFC group are from a feature branch (not the main branch) in a personal fork to the main branch in the ARFC fork of a repository. PRs made in the ARFC organization should:
- Have GitHub Labels (Difficulty:1-Beginner, Status:1-New, etc.)
- Have a GitHub Project (Meta, Fuel Cycles, etc.)
- Have a description in GitHub
- List requested reviews (usually one or two, consider if more than two is really necessary for your request)
- Link to an issue
- Be small (achieve one thing; “atomic”)
Reviewing a Pull Request
Below is a checklist the PR reviewer should
consider before approving a pull request. As a reminder, the
ARFC Code of Conduct still applies
in pull request reviews. Reviews should be both thorough and
respectful. Comments can be made and changes can be requested for a specific
line or lines and the reviewer can even write their own code to improve the
committed files using the
If you are about to review a PR, consider copying and pasting this checklist into your review comment and checking the boxes as you go along.
- [ ] 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](https://i-share-uiu.primo.exlibrisgroup.com/discovery/fulldisplay?docid=alma99944155312205899&context=L&vid=01CARLI_UIU:CARLI_UIU&tab=LibraryCatalog&lang=en)) - [ ] Is there enough documentation? - [ ] Does the programming style meet the requirements of the repository ([PEP8](https://www.python.org/dev/peps/pep-0008/) for python, [google C++ style guide](https://google.github.io/styleguide/cppguide.html), 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 continuous 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? - [ ] Make a review, leaving kind comments and suggesting changes where needed (to resolve the above). - [ ] Has the author resolved all of the comments and suggestions in your review? - [ ] When you approve of the PR, merge and close it. - [ ] Does this PR close an issue? If so, be sure to descriptively close this issue when the PR is merged - [ ] Thank the author for their contribution.
Responding to Pull Request Reviews
GitHub has posted some excellent advice on responding to pull request feedback. Below are the basic steps.
- [ ] Read the review comments. These can include general comments on the technical substance, documentation, performance, clarity, reproducibility, coding style, and even the formatting of the submission. - [ ] Some comments contain commit suggestions. These comments can either be discussed in the comment box or committed directly from the review with the `commit suggestion` button. With this button, the suggested changes are committed and pushed immediately to the PR branch, which makes incorporating suggested changes faster. - [ ] For other comments, discuss the suggestion in the comment box and either make commits that directly satisfy it or eventually resolve it in some other way. - [ ] As you make commits, push them to your pull request branch. - [ ] All reviewer comments should be incorporated, responded to, handled, and/or discussed further. Do not leave comments un-handled. - [ ] Once you have handled all comments, it's wise to alert the reviewers that the PR is ready to be re-reviewed. It's best practice to make a comment in the PR and to re-request a review. To request a fresh review from a reviewer, in the sidebar of the Conversation tab, click the refresh icon. - [ ] Once the PR has been approved by at least one reviewer, a reviewer will merge the changes and the PR will be closed.