Does anyone have a document they follow for having guidelines reviewing code within a team?
I read through this document today and it was helpful for this task:
https://github.com/mawrkus/pull-request-review-guide?tab=readme-ov-file
Different ways developers can review a pr
- sanity check (confirm it looks good and logs are on the pr, tests are written or a checklist followed)
- devs should check this themselves but it sometimes slips.
- reviewing a/c's
- devs should check this themselves but it sometimes slips.
- an extensive manual test
- usually a developer asks for another pair of eyes on a manual regression test.
- comment with approvals a code suggestion
- hold the line, if someone is learning a new pattern and deviating significantly from that pattern.
- other ways to review ...
- the point is different pr's should be evaluated differently
- the goal is to help inform the person making the pr
- the person who made the pr is responsible for the change.
- usually reviewing a pr you are there to assist and help them be informed.
- the goal is to help inform the person making the pr
If you do not know how to review a pr. Ask the person who submitted it, how they want it to be reviewed.
When should a pr be declined or reassigned?
- What is your current measure of when a pr should not be merged?
(My measure: If there is a known breaking change) - What is your current measure of when a pr should be reviewed by someone else?
(My measure: If a significant change is presented on something someone else worked on like water system group) - What is your current measure of when a pr should be reviewed by only the lead developer?
(My measure: rarely, maybe only assign the lead to evaluate a huge refactor)
More considerations regarding pr's:
- how many users are impacted by your codebase in general.
- how many users are impacted by your change?
- how far away are you from a release?