Compendium of the best non-written Pull-Request, or Merge-Request, etiquette rules
These rules they 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.
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 use.
Pull-Request manners
- Ask questions; don't make demands.
- Ownership of code: no one owns the code, Company does.
- Conflicts are part of the job and will always exist: Solve conflicts as a Team.
- 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 use.
- Explain your reasons with examples to simplify the suggested idea in advance while offer ways to simplify or improve.
- Try to respond to every comment.
Pull-Request persons envolved
- 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.
- Don't use sarcasm, be polite and avoid using derogatory terms.
- Be humble. Humility, it is very important 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 hard, GitLab PR line even harder.
- Code reviewers should provide feedback, not to be a gatekeeper.
Pull-Request Conflicts
- 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-Rquest 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".
๐ฏ Thanks!
Now, don't be an stranger. Let's stay in touch!
Top comments (2)
A lot of this article falls under the "mentorship" section we cover in our similar guide to Pull Requests for those in a position to teach others.
For everyone else, it's crank 'em out time relying on the bots to keep things orderly. Thanks for the article!
Fantastic read, Leo! ๐ Your point about keeping pull requests small and manageable really resonated with me. Itโs a practice that not only respects the reviewer's time but also significantly enhances the quality of code reviews by focusing on clarity and precision. Implementing this approach has transformed my team's review process, leading to more efficient and effective collaboration. Your article is a timely reminder of the importance of etiquette in pull requests, which is fundamental in fostering a productive and respectful coding environment. Thanks for sharing these valuable insights!