IM8 Policy: SC-3: Peer Review
Require peer review and approval by a designated reviewer before merging into the default branch.

Merge Request

The Trunk-Based Development workflow provides a gating mechanism to send new changes to a peer reviewer before merging them back into the main trunk for deployment. Source code repositories like SHIP-HATS Gitlab can be configured to disallow pushing directly to the main branch, and force developers to submit a merge request to a peer for review first.

IM8 Policy: SD-2: Default Branch Push Permissions
Configure the code repository to prevent pushes (including force pushes) to the default branch.

It’s important for designated reviewers to act on merge requests in a timely fashion, to avoid the submitter being blocked or building on code that may be changed based on review feedback. If merge requests are left open for a long time, they effectively take on the same overhead as long-running branches.

Quality Metric: Merge Request Cycle Time
Low is good, indicating timely review feedback. Too low may indicate not enough time was given for a thorough review.

To avoid wasting a reviewer’s time and automate some of the review process, merge requests can also trigger the CI pipeline, making sure all tests and quality gates pass before passing it over to look at.

IM8 Policy: SD-3: Continuous Integration (CI) Tests
Require Continuous Integration (CI) tests to pass before merging into the default branch.

Code Review Guidelines

What to look for

We will assume that as much as possible has been automated and shifted left to before the reviewer receives the request to review the code - so it already conforms to the team’s agreed style, and its basic correctness is validated and documented by tests. So a human team member can add their value on top of that.

Some things to look for:

1. Correctness

• Does the code do what it is intended to do?
• Are all requirements from the related user story, ticket, or bug addressed?
• Are there edge cases or failure conditions that are unaccounted for?

2. Clarity and Readability

• Is the code easy to understand?
• Are variable, function, and class names descriptive and meaningful?
• Could a future developer (or your future self) understand what’s going on without explanation?

3. Simplicity and Design

• Is the solution as simple as possible for the given problem?
• Are there unnecessary abstractions, duplication, or complexity?
• Is the logic well-structured and modular?

4. Consistency

• Does the code follow existing patterns and conventions in the codebase?
• Are naming, formatting, and structure consistent with team or project standards?
• Is the code aligned with architectural decisions or domain models?

5. Tests and Test Coverage

• Are there automated tests included for new or changed functionality?
• Do the tests cover normal behavior, edge cases, and failure modes?
• Are the tests themselves easy to understand and maintain?

6. Security and Data Handling

• Are sensitive operations (e.g., authentication, authorization, data access) handled securely?
• If applicable, is input validated and output escaped?
• Is sensitive or user-specific data handled in accordance with privacy requirements?
• See also Secure Code Review Guidelines

7. Performance Considerations

• Are there obvious performance issues (e.g., unnecessary loops, large queries, blocking calls)?
• Does the change introduce potential scalability concerns?

8. Impact on Other Areas

• Could this change unintentionally affect other parts of the system?
• Have potential side effects been considered?
• Are there migrations, deployments, or config changes that need to be coordinated?

9. Documentation

• Is documentation updated where needed (e.g., code comments, README files, API specs)?
• Are assumptions, trade-offs, or complex logic explained clearly?

10. Merge Request Hygiene

• Is the MR description clear and informative?
• Are commit messages meaningful and relevant?
• Is the MR appropriately scoped (not too large or unfocused)?
• Is it tagged with any User Story or backlog item ID?

Giving feedback

When leaving comments, aim to be constructive and collaborative. Asking questions (e.g., “What would happen if…?” or “Could we simplify this by…?”) invites discussion and avoids sounding overly critical. Focus on the code, not the person.

Not every comment needs to be a blocker. Use suggestions for optional improvements and reserve blocking comments for issues that genuinely impact correctness, clarity, or maintainability. Agree on when it’s acceptable to merge with follow-up tasks (e.g., TODOs or tech debt tickets). There is even a standard for conventional comments, similar to that for tagging commits.

When a change is complex or sensitive, consider a synchronous walkthrough or pair review. This helps accelerate understanding, align on intent, and resolve issues quickly.

Summary

A good code review workflow is about more than gatekeeping — it’s a collaborative practice that improves code quality, supports shared ownership, and helps teams grow together. With consistent, respectful, and thoughtful reviews, teams can maintain a healthy, productive development culture.