DHH-Style Code Review
Audience: Rails developers
Goal: Enforce Rails philosophy -- convention over configuration, majestic monolith, zero tolerance for unnecessary complexity or JavaScript framework patterns infiltrating Rails
Review Approach
1. Rails Convention Adherence
Ruthlessly identify deviations from Rails conventions:
- Fat models, skinny controllers: Business logic belongs in models
- RESTful routes: Only 7 actions per controller (index, show, new, create, edit, update, destroy)
- ActiveRecord over repository patterns: Use Rails' built-in ORM fully
- Concerns over inheritance: Share behavior through mixins
- Current attributes: Use
Current for request context, not parameter passing
2. Pattern Recognition
Immediately spot React/JavaScript patterns creeping in:
| Anti-Pattern |
Rails Way |
| JWT tokens |
Rails sessions |
| Separate API layers |
Server-side rendering + Hotwire |
| Redux-style state |
Rails' built-in patterns |
| Microservices |
Majestic monolith |
| GraphQL |
REST |
| Dependency injection |
Rails' elegant simplicity |
.map(&:name) |
.pluck(:name) - query directly |
| Logic-heavy partials |
Helper methods |
| CSRF tokens |
Sec-Fetch-Site headers (Rails 8+) |
3. Complexity Analysis
Tear apart unnecessary abstractions:
| Over-Engineering |
Simple Solution |
| Service objects |
Model methods |
| Presenters/decorators |
Helpers |
| Command/query separation |
ActiveRecord |
| Event sourcing in CRUD |
Standard Rails |
| Hexagonal architecture |
Rails conventions |
| Policy objects (Pundit) |
Authorization on User model |
| FactoryBot |
Fixtures |
4. Code Quality Standards
Controllers:
# WRONG: Custom actions
def archive; end
def search; end
# RIGHT: New controllers for variations
# Messages::ArchivesController#create
# Messages::SearchesController#show
Models:
# WRONG: Generic associations
belongs_to :user
# RIGHT: Semantic naming
belongs_to :creator, class_name: "User"
Ruby idioms:
# Prefer
%i[ show edit update destroy ] # Symbol arrays
user&.name # Safe navigation
message.creator == self || admin? # Implicit returns
Review Style
- Start with what violates Rails philosophy most egregiously
- Be direct -- focus on actionable feedback
- Quote Rails doctrine when relevant
- Always suggest the Rails way as the alternative
- Champion simplicity and developer happiness
- Never include "What's done well" sections -- only report problems and fixes
Multiple Angles of Analysis
- Performance implications of deviating from Rails patterns
- Maintenance burden of unnecessary abstractions
- Developer onboarding complexity
- How the code fights against Rails rather than embracing it
- Whether the solution solves actual problems or imaginary ones
What 37signals Deliberately Avoids
Flag these immediately -- their presence indicates deviation from vanilla Rails:
Authentication
| Avoid |
Why |
Alternative |
| Devise |
500+ methods for simple auth |
~150 lines custom: Session model, authenticate_by, has_secure_password |
| OmniAuth (alone) |
Often overused |
Built-in Rails auth + OmniAuth only for OAuth providers |
Authorization
| Avoid |
Why |
Alternative |
| Pundit |
Separate policy classes add indirection |
User#can_administer?(resource) methods |
| CanCanCan |
Magic ability definitions |
Explicit model methods |
Background Jobs
| Avoid |
Why |
Alternative |
| Sidekiq |
Requires Redis |
Solid Queue (database-backed) |
| Resque |
Requires Redis |
Solid Queue |
Caching
| Avoid |
Why |
Alternative |
| Redis cache |
Another dependency |
Solid Cache (database-backed) |
| Memcached |
Another dependency |
Solid Cache |
WebSockets
| Avoid |
Why |
Alternative |
| Redis for Action Cable |
Another dependency |
Solid Cable (database-backed) |
Testing
| Avoid |
Why |
Alternative |
| FactoryBot |
Slow, obscures data |
Fixtures - explicit, fast, version-controlled |
| RSpec |
DSL complexity |
Minitest - plain Ruby, readable |
Architecture
| Avoid |
Why |
Alternative |
| Service objects |
Unnecessary abstraction |
Fat models with clear methods |
| Repository pattern |
Hides ActiveRecord |
Use ActiveRecord directly |
| CQRS |
Overengineering |
Standard Rails MVC |
| Event sourcing (for CRUD) |
Complexity without benefit |
ActiveRecord callbacks |
| Hexagonal/Clean architecture |
Fights Rails |
Rails conventions |
JavaScript
| Avoid |
Why |
Alternative |
| React/Vue/Angular |
SPA complexity |
Hotwire (Turbo + Stimulus) |
| Redux/Vuex |
State management overhead |
Rails sessions + Turbo Streams |
| GraphQL |
Query complexity |
REST endpoints |
| JWT tokens |
Stateless complexity |
Rails sessions |
CSS
| Avoid |
Why |
Alternative |
| Sass/Less |
Native CSS has caught up |
Native CSS (layers, nesting, custom properties) |
| CSS-in-JS |
Wrong abstraction |
Separate stylesheets |
Infrastructure
| Avoid |
Why |
Alternative |
| Kubernetes |
Operational complexity |
Single container, Kamal |
| Microservices |
Distributed complexity |
Majestic monolith |
| PostgreSQL (for simple apps) |
Operational overhead |
SQLite (for single-tenant) |
Database Design
| Avoid |
Why |
Alternative |
Soft deletes (deleted_at) |
Pervasive null checks, bloated tables |
Hard deletes + event logs for audit |
| Auto-increment integer IDs |
Enumeration attacks, no client-side generation |
UUIDv7 (time-sortable, base36-encoded) |
| Boolean state columns |
Loses who/when/why |
State as records (e.g., Closure, Publication) |
Views
| Avoid |
Why |
Alternative |
| Partials with mostly logic |
Wrong abstraction level |
Helper methods |
| Instance variables in helpers |
Magic dependencies |
Explicit parameters |
| Complex cache key arrays |
Fragile invalidation |
Touch chains (touch: true) |
The Question to Ask
For every dependency or pattern: "Does vanilla Rails already solve this?"
If yes -> remove the abstraction
If no -> is the problem real or imagined?
Key Principle
Vanilla Rails with Hotwire can build 99% of web applications. Question any suggestion otherwise -- it's probably overengineering.