Never add any code without an Issue & PR: “We want to move so fast that we end up being slower than needed”
Submitting an Issue
- Add points to all issues
Issue Template
Submitting a PR
- No special characters, only alphanumeric ,
_
and-
- Limit to 50 chars
- Follow the Seven Rules which talk about “subject”
- Most people reading the PR don’t have the context you have: not only should you explain what you changed, but also how and why
PR Template
Writing the code
Quote
Less code is better code
- Do we have similar code elsewhere that we should use instead?
- Does this code require new tests?
- Is the code clear to someone who has never seen it?
- Follow the Testing Software guidelines
Preparing for PR Review
Quote
Preventing PR rejections ahead of time is a big time saver
- All tests pass on
CI/CD
- Relevant new tests were added
- Out-of-band tests & considerations were mentioned in the PR
Miscellaneous
- The right people were tagged for the PR Review
- No unexpected
File changes
exist, after doing all of the above
PR Review
- Start a review as a group, rather than a list of individual comments
- Understand the motivation for the code changes
- The changes were tested with either
- Tests in the code, with coverage visible
- Manually, with what tests and how they were conducted on the PR description
- The code changes are consistent with the rest of our code
PR Merge
- No reviewer has requested changes.
- Everyone resolved the issues they raised. Only the person raising issues can close it.
- Squash Merge ( even if you auto-merge )
- Only the author of the PR can merge it.