Compendium of the best non-written code-review (Pull-Request or Merge-Request) etiquette rules
These rules encourage a collaborative and constructive approach to reviewing code within a team. The guidelines provide a helpful framework for effective communication during the code review process.
- [Updated 14/02/2025]
Pull-Request structure
- It is (highly) recommended to have one PR per commit Either squash your commits to a single commit or Amend the last commit every time you add a new commit.
- Pull requests are not the place for (very) long architecture discussions: Focus on what's important.
- Keep it simple: Small ticket = Small Pull-Requests.
- Having tools like SonarQube, TSLint, Prettier, hooks, etc also helps to speed up PR/MR. These are tools to encourage best practices and security code that are already agreed to be used.
Pull-Request manners
- Ownership of code: No one owns the code; the Company does.
- Every pull request is an opportunity to share knowledge and demonstrate practical skills. Approach it with humility and openness to discussion.
- Ask questions; don't make demands.
- Conflicts are part of the job and will always exist: Solve conflicts as a Team. Avoid Ego.
- Having tools like SonarQube, TSLint, Prettier, hooks, etc also helps to speed up PR/MR. These are tools to encourage best practices and security code that are already agreed to be used.
- Explain your reasons with examples to simplify the suggested idea in advance while offering ways to simplify or improve.
- Respond to every comment, be agile and pro-active, more importantly, look for a quick positive resolution.
Pull-Request persons envolved
- We are all reviewers; we are invited to review, and we should and must act proactively.
- Don't make it a teaching session.
- Don't make it a blaming game.
- Don't be too serious. Pull requests are an open "short" discussion with your team members.
- Avoid passive-aggressive comments, always keep positive.
- Don't use sarcasm, be polite and avoid using derogatory terms.
- Be humble. Humility it is crucial in both submitting and reviewing code
- Be grateful for the reviewer's suggestions. ("Nice Spotted! I'll make that change.")
- Be understanding. Communication online (like using Slack) is complex, GitLab PR line is even harder.
- Code reviewers should provide feedback, not to be a gatekeeper.
Pull-Request Conflicts
- Your PR review is not about you.
- Always keep in mind: assume the best intention from the reviewer's comments.
- If the discussion on the pull-request is becoming long and doesn’t seem helpful for others, it is better to resolve it in-person, or in a different line, and close it with a summary on the pull-request.
- Acknowledgment of Difficulty: Be aware of how hard it is to convey emotion online and how easy it is to misinterpret feedback. If a review seems aggressive or angry or otherwise personal, consider if it is intended to be read that way and ask the person for clarification of intent, in person if possible.
- If you disagree strongly, consider giving it a few minutes before responding; think before you react.
- Gratitude: Remember always please and thank-you as, even with or without conflicts, there is another co-workers giving us their time to approve a PR.
- Whenever there's a disagreement, we should seek a third person's opinion.
Pull-Request Golden Rule
- Pull requests should be dynamic and agile, quick and short chat to share knowledge.
- Don't take it personally. The review is of the "code", "not of you".
- It is not the reviewers' responsibility to test the code.
Code Reviews in the Age of AI
- Human interaction: "I fear the day that technology will surpass our human interaction. The world will have a generation of idiots. - Albert Einstein"
- Critical Thinking: AI can be a useful tool for catching errors, suggesting improvements, and ensuring consistency, but it shouldn't replace human judgment and critical thinking (please check an article about this: https://dev.to/leolanese/developing-in-the-ai-age-1k35)
- AI as an Assistant, not a replacement: Use AI to speed up reviews but ensure human reviewers validate and discuss changes; otherwise, more than having false positive/negatives we miss the essence of the pull-request, which is the sharing and learning opportunity.
- Transparency: Let contributors know if AI was used, so they can factor that into the discussion.
- Balance: Encourage human judgment for complex refactors, architecture decisions, and deeper code understanding.
💯 Thanks!
Now, don't be a stranger. Let's stay in touch!
🔘 gitroll: LeoLanese
🔘 Linkedin: LeoLanese
🔘 Twitter: @LeoLanese
🔘 Portfolio: www.leolanese.com
🔘 DEV.to: dev.to/leolanese
🔘 Blog: leolanese.com/blog
🔘 Questions / Suggestions / Recommendations: developer@leolanese.com email me: developer@leolanese.com