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.
