Skip to main content
Version: Next

Code Review Process & Conventions

Disclaimer​

  • We use code reviews as opportunities to mentor developers, guiding best practices and coding standards.

  • We encourage understanding of broader project architecture and problem-solving strategies.

  • We foster environment where developers feel comfortable asking questions and discussing alternative solutions.

  • We emphasize the importance of writing clean, efficient and maintainable code.

  • We set clear coding standards, implement unit tests.

Pull Request Roles​

There are two main actors involved in a Pull Request (PR):

  • PR Owner: The person who worked on the pull request, understands all business requirements, and wrote the code.
  • Reviewer: The person responsible for reviewing and approving the code. The reviewer can reject a PR if any of conventions are violated.

Creating & Merging a Pull Request​

1. Code Style​

Pull requests should be formatted according to the official code style.

2. Commit Messages​

We do not enforce any particular commit style (considering we squash & merge our PRs). Ensure the commit message is understandable and clear, especially after re-requesting a review.

3. The Boy Scout Rule​

We encourage leaving the code better than it was. However, PRs should contain only the minimum changes related to the code we have to touch. Avoid full reformatting and changes to unrelated parts of the codebase. A cosmetic PR can be opened later for formatting or similar changes if needed.

4. PR Description and Title​

PR descriptions should follow the provided template. It's important to describe the change introduced and how it was accomplished. The PR owner may add comments to highlight specific details or anticipate questions.

5. Merging a PR​

Our merging policy enables the PR owner to merge the PR only after certain requirements are satisfied (e.g., tests are passing, all conversations are resolved). Pull Requests must be squashed & merged to help create a cleaner and linear git history. Don't forget to change the JIRA ticket status once the PR is merged!

Code Review Best Practices​

For PR Owners:​

  1. Maintain the pull request, respond to questions, and ensure all discussions are closed before merging (do not close ongoing conversations).
  2. If you want to show your work progress to the team or open a PR before sending it for review, create a draft PR.
  3. Conflicts can happen. Once the PR has been reviewed, prefer merging develop into your branch instead of rebasing the feature branch to the HEAD of develop. Rebasing involves rewriting history, which could complicate the second review.
  4. If someone requests changes, make the changes if needed, and re-request review when finished.

For Reviewers:​

  1. Start a conversation by questioning the reason for a specific change instead of directly blocking the PR. If conversation threads become crowded, prefer face-to-face conversations. Always put a final summary of the decision before 'resolving' the conversation.
  2. PRs should have precise, small scopes. Review the PR accordingly.
  3. Highlight but do not require changes for code not directly touched by the PR. Use "nit:" (nit-picking) to indicate non-mandatory suggestions that could improve code readability.

Branch Naming​

It's recommended to name branches as follows:

  • Features, tech debt, and anything with a Jira ticket assigned: <JIRA-TICKET-CODE>-short-description

Outcomes​

This approach results in:

  1. High pull request merging pace
  2. Low probability of code conflicts
  3. Increased feature awareness
  4. Decreased context switching
  5. Faster feature integration
  6. Clear and clean git history
  7. Better accountability of all PRs and their nature