Sentry Code Review
Follow these guidelines when reviewing code for Sentry projects.
Review Checklist
Identifying Problems
Look for these issues in code changes:
-
Runtime errors: Potential exceptions, null pointer issues, out-of-bounds access
-
Performance: Unbounded O(n²) operations, N+1 queries, unnecessary allocations
-
Side effects: Unintended behavioral changes affecting other components
-
Backwards compatibility: Breaking API changes without migration path
-
ORM queries: Complex Django ORM with unexpected query performance
-
Security vulnerabilities: Injection, XSS, access control gaps, secrets exposure
Design Assessment
-
Do component interactions make logical sense?
-
Does the change align with existing project architecture?
-
Are there conflicts with current requirements or goals?
Test Coverage
Every PR should have appropriate test coverage:
-
Functional tests for business logic
-
Integration tests for component interactions
-
End-to-end tests for critical user paths
Verify tests cover actual requirements and edge cases. Avoid excessive branching or looping in test code.
Long-Term Impact
Flag for senior engineer review when changes involve:
-
Database schema modifications
-
API contract changes
-
New framework or library adoption
-
Performance-critical code paths
-
Security-sensitive functionality
Feedback Guidelines
Tone
-
Be polite and empathetic
-
Provide actionable suggestions, not vague criticism
-
Phrase as questions when uncertain: "Have you considered...?"
Approval
-
Approve when only minor issues remain
-
Don't block PRs for stylistic preferences
-
Remember: the goal is risk reduction, not perfect code
Common Patterns to Flag
Python/Django
Bad: N+1 query
for user in users: print(user.profile.name) # Separate query per user
Good: Prefetch related
users = User.objects.prefetch_related('profile')
TypeScript/React
// Bad: Missing dependency in useEffect useEffect(() => { fetchData(userId); }, []); // userId not in deps
// Good: Include all dependencies useEffect(() => { fetchData(userId); }, [userId]);
Security
Bad: SQL injection risk
cursor.execute(f"SELECT * FROM users WHERE id = {user_id}")
Good: Parameterized query
cursor.execute("SELECT * FROM users WHERE id = %s", [user_id])
References
- Sentry Code Review Guidelines