Run effective code reviews. Covers review process design, what to look for, automated checks, review speed, giving constructive feedback, and scaling reviews across teams.
This page describes specific techniques and workflows for conducting effective code reviews in software development. These practices apply when team members review each other's changes before merging into shared branches, typically during pull request or merge request processes.
Reviewers should verify that conditional statements correctly handle all expected paths. The most common failure mode involves missing edge cases in loop termination or exception handling.
// Bad: Missing nil check
func processUser(user *User) error {
return user.Name // Will panic if user is nil
}
// Good: Explicit nil handling
func processUser(user *User) error {
if user == nil {
return fmt.Errorf("user cannot be nil")
}
return nil
}All errors from external calls must be checked and handled appropriately. Silent failures occur when error values are discarded without logging or returning.
# Bad: Ignoring returned errors
def save_data(data):
db.insert(data) # Error not checked
return True
# Good: Proper error propagation
def save_data(data):
result, err = db.insert(data)
if err != nil {
return fmt.Errorf("failed to save data: %w", err)
}
return nilFunctions that open files, database connections, or network sockets must ensure these resources are properly closed. Missing defer statements cause resource leaks.
// Bad: No cleanup
public void processFile(String filename) throws IOException {
FileInputStream fis = new FileInputStream(filename);
// Processing logic here
// fis.close() never called
}
// Good: Proper cleanup
public void processFile(String filename) throws IOException {
try (FileInputStream fis = new FileInputStream(filename)) {
// Processing logic here
} // Automatically closed
}Code changes must include unit tests covering new functionality. The review should confirm that test coverage meets minimum thresholds.
# Example configuration for test coverage enforcement
coverage:
threshold: 80
require_coverage: trueAny modifications to configuration files or environment variables must be verified for correctness and consistency with deployment patterns.
# Example config file validation
[database]
host = "localhost"
port = 5432
ssl_enabled = true
# Ensure required fields are present
required_fields = ["host", "port"]When modifying APIs, verify that all changes maintain backward compatibility or properly document breaking changes.
# HTTP request example showing expected behavior
POST /api/v1/users
Content-Type: application/json
{
"name": "John Doe",
"email": "[email protected]"
}Schema changes require careful review of migration scripts and their impact on existing data.
-- Example migration script review checklist
-- 1. Verify idempotency
-- 2. Check for nullable constraints
-- 3. Ensure indexes are appropriate
-- 4. Validate data type choices
CREATE TABLE users (
id BIGSERIAL PRIMARY KEY,
email VARCHAR(255) NOT NULL UNIQUE,
created_at TIMESTAMP DEFAULT NOW()
);Review performance implications of algorithmic changes or data access patterns.
// Bad: Inefficient O(n²) operation
func findMatches(items []string, targets []string) []string {
var matches []string
for _, item := range items {
for _, target := range targets {
if item == target {
matches = append(matches, item)
}
}
}
return matches
}
// Good: O(n) with map lookup
func findMatches(items []string, targets []string) []string {
targetSet := make(map[string]bool)
for _, target := range targets {
targetSet[target] = true
}
var matches []string
for _, item := range items {
if targetSet[item] {
matches = append(matches, item)
}
}
return matches
}Security vulnerabilities often stem from improper input validation or authentication handling.
# Bad: SQL injection vulnerability
def get_user_by_id(user_id):
query = f"SELECT * FROM users WHERE id = {user_id}"
return db.execute(query)
# Good: Parameterized queries
def get_user_by_id(user_id):
query = "SELECT * FROM users WHERE id = ?"
return db.execute(query, [user_id])Review comments should reference exact lines of code and suggest concrete improvements.
## Review comment on line 15
This function could benefit from additional error handling. Consider adding a check for nil values before accessing fields.
```go
if user == nil {
return fmt.Errorf("user cannot be nil")
}
### Use Clear Review Statuses
Use standard review statuses to communicate the state of changes.
- `REQUEST_CHANGES`: Requires modifications before merging
- `APPROVE`: Ready to merge with current changes
- `COMMENT`: General feedback without requiring changes
## Common Patterns for Effective Reviews
### Incremental Review Process
Break large changes into smaller, focused commits that can be reviewed individually.
```bash
# Example workflow
git checkout -b feature/endpoint-refactor
git add --patch
git commit -m "Refactor user endpoint logic"
git add --patch
git commit -m "Add input validation"
git push origin feature/endpoint-refactorEstablish regular review windows to maintain consistent feedback flow.
# Example team workflow configuration
review_schedule:
# Review requests due by Friday 5 PM
review_deadline: "Friday 5:00 PM"
# Minimum 2 reviewers required for critical changes
minimum_reviewers: 2Ensure that code changes include corresponding documentation updates.
## API Endpoint Documentation
### POST /api/v1/users
Creates a new user account with provided details.
#### Request Body
```json
{
"name": "string",
"email": "string"
}
## Failure Modes and Pitfalls
### Silent Configuration Errors
Configuration files can silently fail when required fields are missing or invalid values are used.
```yaml
# Example of problematic configuration
database:
# Missing host field - causes runtime panic
port: 5432
ssl: trueInconsistent variable names across functions cause readability issues and increase maintenance burden.
// Inconsistent naming
func ProcessUser(u *User) error {
return u.Email // Field accessed directly
}
func process_user(user *User) error {
return user.Name // Different casing
}Changes that lack tests can introduce regressions without detection.
# Example test command that should pass
go test ./... -v
# If any test fails, the review should request changesAdding new dependencies requires verification of version compatibility and license compliance.
{
"dependencies": {
"lodash": "^4.17.21",
"express": "^4.18.2"
}
}Configure automated checks to run before code submission.
# Example pre-commit hook configuration
repos:
- repo: https://github.com/golangci/golangci-lint
rev: v1.50.0
hooks:
- id: golangci-lint
args: [--timeout=30s]Ensure code review process integrates with CI systems to validate changes.
# Example CI validation command
make test
make lint
make security-scanRun static analysis tools to catch issues before human review.
# Example static analysis commands
golangci-lint run ./...
eslint --fix src/
rubocop --auto-correctThe key to successful code reviews is maintaining focus on correctness, maintainability, and consistency. Each review should result in improved code quality that benefits the entire team's productivity.
This page was rewritten on 15 October 2026. It replaced a templated version that was not specific to its own title. How these pages are written and checked. If anything here is wrong, tell us at [email protected] and we will correct it.
We use cookies for analytics (Google Analytics) and advertising (Google AdSense) to improve your experience and support free content. Privacy Policy