-
Notifications
You must be signed in to change notification settings - Fork 7
Pull Requests
Procedure for submitting and merging PRs (pull requests).
- Create a pull request
- The title of the PR becomes the commit message in the main branch, so be as concise and specific as possible. Refer to How to Write a Git Commit Message.
- Try to limit to 300 lines of code changes; smaller logical chunks of code is better; https://blog.logrocket.com/using-stacked-pull-requests-in-github/
- https://opensource.com/article/18/6/anatomy-perfect-pull-request
- https://developers.google.com/blockly/guides/modify/contribute/write_a_good_pr
- Add a minimum of 2 reviewers or post in Slack channel #benefits-vro-engineering for anyone to review
- Exception: Dependabot PRs are authored by Dependabot. This means that the two reviewers are the Dependabot Lead for the sprint, and another teammate. See Dependabot for more information.
- Look for successful testing steps as GitHub actions for your changes. Note that some actions such as Container Health Checks will be skipped on some PRs, skipped actions are acceptable but failed actions should be addressed.
- Add comments to your code under the "Files Changed" tab to explain complex logic or code
- Reviewers provide feedback, clearly marking blocking and non-blocking suggestions
- Address feedback and repeat review step until PR is approved
- Squash and merge PR, which will trigger among other things
Code reviews are intended to help all of us grow as engineers and improve the quality of what we ship. These guidelines are meant to reinforce those two goals.
PRs are currently auto assigned to Github department-of-veterans-affairs/benefits-vro-engineers team, this can be tweaked and changed when additional code based rules apply based on the documentation here: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
Aim to respond to code reviews within 1-2 business day.
Remember to highlight things that you like and appreciate while reading through the changes, and to make any other feedback clearly actionable by indicating if it is optional preference, an important consideration, or an error.
Don't be afraid to comment with a question, or to ask for clarification, or provide a suggestion, whenever you don’t understand what is going on at first glance — or if you think an approach or decision can be improved. Code reviews give us a chance to learn from one another, and to reflect, iterate on, and document why certain decisions are made.
Once you're ready to approve or request changes, err on the side of trust. Send a vote of approval if the PR looks ready except for trivial changes. Use "Request changes" when there's a blocking issue or major refactors that should be done.
It is highly recommended that you create a Draft Pull Request initially, then when ready, mark it as Ready for Review.
Your PR should be small enough that a reviewer can reasonably respond within 1-2 business days. For larger changes, break them down into a series of PRs. If refactors are included in your changes, try to split them out as recommended below.
As a PR writer, you should consider your description and comments as documentation; current and future team members will refer to it to understand your design decisions. Include relevant context and business requirements, and add preemptive comments (in code or PR) for sections of code that may be confusing or worth debate.
After you make requested changes in response to code review feedback, please re-request reviews from the reviewers to notify them that the work is ready to be reviewed again.