Skip to content

CodeReviewGuidelines

Torgeir Holen edited this page Sep 19, 2023 · 1 revision

Code Review Guidelines

link: https://github.com/Altinn/altinn-studio-docs/blob/master/CONTRIBUTING.md#internal-code-review-guidelines

Internal Code Review Guidelines

General Principles:

  1. Avoid terms that might be interpreted as personal attacks (e.g., "idiotic" or "useless"). Assume that everyone is making their best effort.

  2. Be moderate in your language. Avoid strong exaggerations like "always," "never," etc.

  3. We're all on the same team. The intention is not to criticize but to produce better code.

  4. Smaller, frequent pull requests are easier to manage than larger ones. Aim to limit the scope of code to approximately 200 lines for each review.

Guidelines for Reviewers:

  1. Focus on the Code, Not the Person: The goal is to elevate the quality of the code, not to showcase your expertise. Be constructive in your feedback.

  2. Adhere to Established Standards: Stick to coding conventions and practices that are accepted within the team. Deviations must be justified.

  3. Understand the Purpose: Make sure to understand the rationale behind the code you're reviewing. Go through any associated documentation or task definitions.

  4. Clarify Importance: Distinguish between critical issues and lesser suggestions. Make it clear what needs to be amended before the code can be merged.

  5. Invite Discussion: Ask questions like, "Have you considered isolating this logic into a separate method?" rather than giving direct instructions.

Guidelines for Authors:

  1. Self-Check: Review your own code to catch simple errors before requesting a review.

  2. Details in Pull Requests: Include an explanatory text that provides reviewers with necessary context.

  3. Respond to Reviews: Provide feedback on all comments. A simple "Done" or "Agreed" is often sufficient.

  4. Be Receptive: Be open to critique and suggestions. Argue your case if needed, but be willing to consider alternatives.

Procedural Rules:

  1. Speed is a Virtue: The review should be quick to avoid potential conflicts during merging.

  2. More Eyes, Better Outcome: Aim to have at least two reviewers for substantial code changes.