Merge Request Best Practices

Guidelines for creating well-structured, reviewable GitLab merge requests (MRs) that facilitate effective code review and knowledge sharing.

GitLab Terminology

GitLab uses "Merge Request" (MR) instead of "Pull Request" (PR). These terms are interchangeable, but we'll use "MR" to match GitLab's terminology.

Overview

Merge requests are the primary mechanism for code review, quality assurance, and knowledge transfer in GitLab. Well-crafted MRs make reviews faster, catch more bugs, and improve team collaboration.

Core Principles

Small and Focused: MRs should address a single concern or feature
Self-Documenting: Description and code should tell a clear story
Evidence-Based: Include proof that changes work as intended
Reviewer-Friendly: Make it easy for reviewers to understand and verify changes
Timely: Create MRs early and respond to feedback quickly
Link to User Story: Every MR must link to its Jira ticket

MR Sizing Guidelines

Size Targets

Size	Lines Changed	Review Time	When to Use
Tiny	1-50	5-10 mins	Bug fixes, config changes, documentation
Small	51-200	15-30 mins	Single feature, refactoring one component
Medium	201-500	30-60 mins	Feature with tests, multiple related changes
Large	501-1000	1-2 hours	Complex feature, architectural change
Huge	1000+	2+ hours	Avoid! Break into smaller PRs

Recommended Size

Aim for small to medium PRs (50-500 lines). They get reviewed faster, have fewer bugs, and are easier to understand.

Breaking Down Large Changes

When facing a large change:

Create a feature branch for the entire feature
Break into logical chunks:
- PR 1: Data model changes
- PR 2: Repository/DAO layer
- PR 3: Service layer
- PR 4: API endpoints
- PR 5: Frontend integration
- PR 6: Tests and documentation
Use draft PRs to show progress without requesting review
Stack PRs when dependencies exist (clearly document in description)

Stacked PRs

When creating stacked PRs, clearly document the dependency chain in each PR description:

**Depends on**: #123, #124
**Review order**: Review dependencies first

PR Title Format

Use consistent, descriptive titles following conventional commits:

[JIRA-1234] feat: Add payment retry mechanism for failed transactions
[JIRA-5678] fix: Resolve race condition in account balance updates
[JIRA-9012] refactor: Extract payment validation logic to service layer
[JIRA-3456] docs: Update API documentation for payment endpoints

Title Format

[TICKET-ID] type: Short description (max 72 characters)

Commit Types

feat: New feature
fix: Bug fix
refactor: Code restructuring without behavior change
perf: Performance improvement
test: Adding or updating tests
docs: Documentation updates
style: Code formatting (no logic changes)
chore: Build, dependencies, tooling
security: Security fixes or improvements
revert: Reverting previous changes

PR Description Template

Required Sections

Every PR must include a comprehensive description:

## Summary

Brief overview of what this PR does and why (2-3 sentences).

## Jira Ticket

[JIRA-1234](https://jira.company.com/browse/JIRA-1234)

## Problem Statement

Describe the problem or requirement this PR addresses:
- What was broken or missing?
- Why is this change necessary?
- What is the business impact?

## Solution Approach

Explain your implementation approach:
- High-level design decisions
- Key architectural choices
- Alternatives considered (and why rejected)
- Any trade-offs made

## Technical Details

### Changes Made

- Updated `PaymentService` to implement retry logic
- Added `RetryConfig` with exponential backoff
- Implemented circuit breaker pattern for external API calls
- Added comprehensive error handling

### Database Changes

- [ ] No database changes
- [ ] Added migration script (see `V1.2.3__add_retry_table.sql`)
- [ ] Backwards compatible
- [ ] Requires downtime: No

### API Changes

- [ ] No API changes
- [ ] New endpoint: `POST /api/payments/retry`
- [ ] Modified endpoint: `GET /api/payments/{id}` (added retry status)
- [ ] Backwards compatible
- [ ] Updated OpenAPI spec

## Testing Evidence

### Unit Tests

```bash
✓ PaymentServiceTest.shouldRetryFailedPayment
✓ PaymentServiceTest.shouldStopAfterMaxRetries
✓ PaymentServiceTest.shouldUseExponentialBackoff
✓ CircuitBreakerTest.shouldOpenAfterFailures

Coverage: 92% (target: 85%)

Integration Tests

✓ PaymentIntegrationTest.shouldRetryOnNetworkFailure
✓ PaymentIntegrationTest.shouldNotRetryOnValidationError

All integration tests passing (8/8)

Manual Testing

Tested scenarios:

[GOOD] Payment succeeds on first attempt
[GOOD] Payment fails once, succeeds on retry
[GOOD] Payment fails multiple times, circuit breaker opens
[GOOD] Circuit breaker recovers after timeout

Screenshots/Evidence

Before: Before Screenshot

After: After Screenshot

Logs:

2025-10-25 10:15:23 INFO  PaymentService - Attempting payment PAY-123
2025-10-25 10:15:24 WARN  PaymentService - Payment failed, retrying (attempt 1/3)
2025-10-25 10:15:26 INFO  PaymentService - Payment succeeded on retry

Performance Impact

No performance impact
Improved performance (explain below)
Potential performance degradation (explain below)

Performance test results:

Baseline: 200ms average, 500ms p99
After change: 210ms average, 520ms p99
Impact: +5% latency (acceptable for reliability improvement)

Security Considerations

No security impact
Security improvement (explain below)
Requires security review

Security checklist:

[GOOD] Input validation added
[GOOD] No sensitive data logged
[GOOD] Authorization checks in place
[GOOD] OWASP Top 10 reviewed

Rollout Plan

Can be deployed immediately
Requires feature flag: payment.retry.enabled
Requires configuration: See application.yml updates
Requires gradual rollout: 10% → 50% → 100%

Rollback Plan

If issues occur:

Disable feature flag payment.retry.enabled=false
Revert deployment to previous version
No data migration rollback needed

Checklist

Before Requesting Review

Review Requirements

Requires 1 reviewer (feature PR)
- Requires 2 reviewers (release PR)
Requires code owner approval
Requires security team review
Requires architecture review

Dependencies

No dependencies
Depends on PR #XXX (describe relationship)
Blocked by external dependency (describe)

Follow-up Work

No follow-up needed
Follow-up tracked in JIRA-XXXX (describe)

Questions for Reviewers

Specific areas where you want feedback:

Is the retry configuration too aggressive?
Should we add metrics for retry attempts?
Any edge cases I missed?

Reviewers: @teammate1 @teammate2 Estimated Review Time: 30 minutes Labels: enhancement, backend, payments

---

## Evidence Requirements

### All PRs Must Include

1. **Test Results**
   - Unit test output showing passing tests
   - Coverage report (aim for >85%)
   - Integration test results

2. **Code Quality**
   - Linting passed (zero errors/warnings)
   - Sonar quality gate passed
   - No security vulnerabilities (SAST scan)

### Feature PRs Must Include

3. **Functional Evidence**
   - Screenshots or screen recordings
   - API request/response examples (Postman, curl)
   - Logs showing feature working correctly

4. **Edge Cases**
   - Evidence of error handling
   - Boundary condition testing
   - Negative test cases

### Bug Fix PRs Must Include

5. **Bug Reproduction**
   - Steps to reproduce the bug
   - Screenshot/logs showing the bug
   - Explanation of root cause

6. **Fix Verification**
   - Evidence bug no longer occurs
   - Regression test added

### Performance Changes Must Include

7. **Performance Metrics**
   - Baseline measurements
   - Post-change measurements
   - Gatling/JMeter test results
   - P50, P95, P99 latencies

---

## PR Workflow

### 1. Create Feature Branch

```bash
# From main branch
git checkout main
git pull origin main

# Create feature branch
git checkout -b feature/JIRA-1234-payment-retry

# Or for bug fix
git checkout -b bugfix/JIRA-5678-balance-race-condition

2. Develop with Small Commits

# Make logical, atomic commits
git add src/services/PaymentService.java
git commit -m "feat: Add retry logic to PaymentService"

git add src/config/RetryConfig.java
git commit -m "feat: Add retry configuration"

git add src/test/services/PaymentServiceTest.java
git commit -m "test: Add tests for payment retry logic"

3. Push and Create PR

# Push to remote
git push origin feature/JIRA-1234-payment-retry

# Create PR via GitHub/GitLab UI or CLI
gh pr create --title "[JIRA-1234] feat: Add payment retry mechanism" \
             --body "$(cat .github/pull_request_template.md)"

4. Address Review Comments

# Make changes based on feedback
git add .
git commit -m "refactor: Extract retry config to separate class"

# Push updates
git push origin feature/JIRA-1234-payment-retry

# If asked to squash commits
git rebase -i main
git push --force-with-lease origin feature/JIRA-1234-payment-retry

5. Merge

# Once approved, squash and merge via UI
# Or via command line
git checkout main
git pull origin main
git merge --squash feature/JIRA-1234-payment-retry
git commit -m "[JIRA-1234] feat: Add payment retry mechanism"
git push origin main

# Delete feature branch
git branch -d feature/JIRA-1234-payment-retry
git push origin --delete feature/JIRA-1234-payment-retry

PR State Management

Draft PRs

Use draft PRs to:

Show work in progress
Get early feedback on approach
Demonstrate progress to stakeholders
Run CI pipeline before formal review

## Work in Progress

This PR is not ready for review yet.

### Completed
- [x] Added retry logic
- [x] Unit tests

### TODO
- [ ] Integration tests
- [ ] Documentation
- [ ] Performance testing

**ETA for review**: Tomorrow EOD

Ready for Review

Convert to ready when:

All code is complete
Tests are passing
Documentation is updated
Self-review is done
PR description is complete

Changes Requested

When reviewers request changes:

Address all comments (or explain why not)
Re-request review after pushing changes
Mark conversations as resolved
Provide evidence changes work

Approved

After approval:

Verify CI pipeline passes
Check for merge conflicts
Squash commits if needed
Merge using team's preferred strategy

Common PR Anti-Patterns

The Monster PR

Files changed: 143
Lines added: 8,234
Lines removed: 3,421
Review time: "I need a week"

Problem: Too large to review effectively

Solution: Break into smaller, logical PRs

The "Trust Me" PR

## Summary
Added new feature

## Testing
Tested locally, works fine

Problem: No evidence, no context

Solution: Include comprehensive description and evidence

The WIP Disguised as Ready

✗ 15 test failures
✗ 43 linting errors
✗ Build failing
Status: Ready for Review

Problem: Wasting reviewers' time

Solution: Use draft PRs until actually ready

The Silent Treatment

Reviewer: "Can you explain this logic?"
*3 days pass with no response*

Problem: Stalls team progress

Solution: Respond to reviews within 24 hours

The Drive-By Review Request

@entire-team please review ASAP

Problem: Unclear who's responsible

Solution: Tag 1-2 specific reviewers

MR Templates

Create .gitlab/merge_request_templates/default.md:

## Summary
<!-- Brief overview of changes -->

## Jira Ticket
<!-- [JIRA-XXXX](https://jira.company.com/browse/JIRA-XXXX) -->

## Problem Statement
<!-- What problem does this solve? -->

## Solution Approach
<!-- How did you solve it? -->

## Testing Evidence
<!-- Paste test results, screenshots, etc. -->

## Checklist
- [ ] Tests added/updated
- [ ] Documentation updated
- [ ] No linting errors
- [ ] Self-reviewed
- [ ] Security considered
- [ ] Linked to Jira ticket

GitLab-Specific Features

Merge Request Settings

Project Settings > Merge Requests:

Merge Method:

[GOOD] Merge commit: Creates merge commit (preserves branch history)
[GOOD] Squash commits: Combine all commits into one before merging
[BAD] Fast-forward merge: Only for simple changes (use sparingly)

Merge Checks (required):

Pipelines must succeed
All threads must be resolved
Require approval from code owners
Allow merge commits (depends on strategy)

Squash Commits:

Encourage (recommended for feature branches)
Default message template:

[JIRA-1234] feat: Add payment retry mechanism

Co-authored-by: Reviewer Name <[email protected]>

Delete Source Branch:

Automatically delete source branch after merge
Keeps repository clean
Branches available in "Deleted branches" if needed

Approval Rules

Settings > Merge Request Approvals:

Feature MRs:

Approvals required: 1
Eligible approvers: Any team member
Cannot be author
Cannot be committer

Release MRs (to main or release/*):

Approvals required: 2
Eligible approvers: Tech leads or senior developers
Code owner approval required
Security team approval (if flagged)

Prevent Approval by Author:

# .gitlab-ci.yml
rules:
  approval_settings:
    prevent_author_approval: true
    require_password_to_approve: false
    approvals_before_merge: 1

Code Owners (.gitlab/CODEOWNERS):

# Backend code
/src/main/java/**          @backend-team @tech-lead-java
/src/main/resources/**     @backend-team

# Frontend code
/src/app/**                @frontend-team @tech-lead-react
/src/components/**         @frontend-team

# Security-sensitive
/src/**/security/**        @security-team
/src/**/auth/**            @security-team

# Database migrations
/src/main/resources/db/migration/**  @database-team @tech-lead-java

# CI/CD
/.gitlab-ci.yml            @devops-team
/Dockerfile                @devops-team

# Documentation
/docs/**                   @tech-writers
/README.md                 @tech-writers

Merge Request Labels

Automatic Labeling (Settings > Labels):

Create and use labels consistently:

Type: feature, bugfix, hotfix, refactor, docs
Component: backend, frontend, mobile, infrastructure
Priority: critical, high, medium, low
Status: wip, ready-for-review, changes-requested, approved
Area: payments, authentication, reporting

Label Automation (.gitlab-ci.yml):

add-labels:
  stage: .pre
  script:
    - |
      if [[ "$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME" == feature/* ]]; then
        glab mr update $CI_MERGE_REQUEST_IID --label "feature"
      elif [[ "$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME" == bugfix/* ]]; then
        glab mr update $CI_MERGE_REQUEST_IID --label "bugfix"
      fi
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'

Protected Branches

Settings > Repository > Protected Branches:

main branch:

Allowed to merge: Maintainers only
Allowed to push: No one (force MR workflow)
Allowed to force push: No
Code owner approval required: Yes
Require approval: 2 approvals

develop branch:

Allowed to merge: Developers + Maintainers
Allowed to push: No one
Allowed to force push: No
Require approval: 1 approval

release/* branches:

Allowed to merge: Maintainers only
Allowed to push: No one
Allowed to force push: No
Require approval: 2 approvals

Merge Request Pipelines

Pipeline Must Pass:

# .gitlab-ci.yml
workflow:
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'

stages:
  - lint
  - test
  - build
  - quality-gate

lint:
  stage: lint
  script:
    - npm run lint
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'

test:
  stage: test
  script:
    - ./gradlew test
  coverage: '/Total.*?([0-9]{1,3})%/'
  artifacts:
    reports:
      junit: build/test-results/test/**/TEST-*.xml
      coverage_report:
        coverage_format: cobertura
        path: build/reports/cobertura-coverage.xml
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'

quality-gate:
  stage: quality-gate
  script:
    - |
      COVERAGE=$(cat coverage-summary.json | jq '.total.lines.pct')
      if (( $(echo "$COVERAGE < 85" | bc -l) )); then
        echo "Coverage $COVERAGE% is below 85%"
        exit 1
      fi
  allow_failure: false
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'

Draft Merge Requests

Mark as Draft:

Prefix title with Draft: or WIP:
Prevents accidental merging
CI pipeline still runs
Can request early feedback

Convert to Draft:

# Via glab CLI
glab mr update 123 --draft

# Via UI
Click "Mark as draft" button

When to Use Draft MRs:

Work in progress, not ready for review
CI pipeline verification needed
Early feedback on approach
Demonstrate progress to stakeholders

Merge Request Threads

Creating Discussions:

Click line number in diff
Add comment
Mark as blocking issue or suggestion

Thread Resolution:

Author or reviewer can resolve
All threads must be resolved before merge (enforced)
Use "Resolve thread" button
Can reopen if needed

Thread Types:

**Blocking Issue**: This will cause a bug in production
**Suggestion**: Consider using Optional.ofNullable() here
**Question**: Why did you choose this approach?
**Nitpick**: Minor formatting issue (non-blocking)
**Praise**: Great solution to this problem! 

Merge Request Dependencies

Link Related MRs:

## Dependencies

**Depends on**:
- !456 (Backend API changes)
- !457 (Database migration)

**Blocks**:
- !459 (Frontend integration)

**Related**:
- !450 (Similar refactoring)

Review Order:

 **Important**: Review MRs in this order:
!456 (Backend API changes) - Review first
!457 (Database migration) - Review second
This MR - Review last

Quick Actions

Use quick actions in MR comments:

/approve                    # Approve MR
/unapprove                 # Remove approval
/assign @username          # Assign to user
/unassign @username        # Unassign user
/label ~feature ~backend   # Add labels
/unlabel ~wip              # Remove label
/close                     # Close MR
/reopen                    # Reopen MR
/merge                     # Merge when pipeline succeeds
/draft                     # Mark as draft
/ready                     # Mark as ready
/spend 2h 30m              # Log time spent
/due 2025-02-15            # Set due date

Merge Request Analytics

Track in GitLab:

Project > Analytics > Merge Request Analytics

Key Metrics:

Time to merge (target: <2 days)
Time to first review (target: <4 hours)
Number of review cycles (target: <3)
MR size distribution
Approval rate

Quality Indicators:

PRs requiring >3 review cycles: Unclear requirements
PRs open >7 days: Stale or blocked
Large PRs (>1000 lines): Need splitting
PRs with failing pipelines >24h: Blocking team

Metrics and Continuous Improvement

Track PR Health

Time to First Review: Target <4 hours
Time to Merge: Target <2 days
Review Cycles: Target <3 iterations
PR Size: Target median <300 lines
Approval Rate: Should be >80% after addressing comments

Red Flags

PRs open >1 week (stale)
PRs with >5 review cycles (unclear requirements)
PRs with >1000 lines (too large)
PRs with 0 tests (low quality)
PRs with failing CI for >24 hours (blocking team)

GitLab MR Workflow Diagram

Summary

Key Takeaways:

Keep MRs small (50-400 lines) for faster, more effective reviews
Write comprehensive descriptions with problem, solution, evidence, and Jira link
Include test results, screenshots, and logs as proof changes work
Respond to reviews promptly (within 24 hours)
Use draft MRs for work in progress
Follow the GitLab MR template for consistency and completeness
Break large changes into logical, reviewable chunks
Configure approval rules (1 approval for features, 2 for releases)
Use protected branches to enforce MR workflow
Leverage GitLab features: Labels, quick actions, code owners, threading

Critical Systems

For business-critical or highly regulated MRs, always include:

Audit logging evidence (where applicable)
Data integrity verification
Rollback plan for critical data changes
Security scan results
Link to Jira ticket with acceptance criteria

Overview​

Core Principles​

MR Sizing Guidelines​

Size Targets​

Breaking Down Large Changes​

PR Title Format​

Title Format​

Commit Types​

PR Description Template​

Required Sections​

Integration Tests​

Manual Testing​

Screenshots/Evidence​

Performance Impact​

Security Considerations​

Rollout Plan​

Rollback Plan​

Checklist​

Before Requesting Review​

Review Requirements​

Dependencies​

Follow-up Work​

Questions for Reviewers​

2. Develop with Small Commits​

3. Push and Create PR​

4. Address Review Comments​

5. Merge​

PR State Management​

Draft PRs​

Ready for Review​

Changes Requested​

Approved​

Common PR Anti-Patterns​

The Monster PR​

The "Trust Me" PR​

The WIP Disguised as Ready​

The Silent Treatment​

The Drive-By Review Request​

MR Templates​

GitLab-Specific Features​

Merge Request Settings​

Approval Rules​

Merge Request Labels​

Protected Branches​

Merge Request Pipelines​

Draft Merge Requests​

Merge Request Threads​

Merge Request Dependencies​

Quick Actions​

Merge Request Analytics​

Metrics and Continuous Improvement​

Track PR Health​

Red Flags​

Further Reading​

GitLab MR Workflow Diagram​

Summary​