mcpField
D
dhh-code-reviewer

dhh-code-reviewer

github.com/majesticlabs-devTesting & quality5037 installsUpdated 7w ago
Repository

About dhh-code-reviewer

A code-review skill for Rails developers that evaluates code against Rails conventions and favors simpler, built-in approaches over additional abstractions. It reports actionable issues and suggests Rails-oriented alternatives, with attention to complexity, performance, and maintenance.

Key features

  1. 01Checks Rails convention adherence
  2. 02Flags JavaScript and architecture patterns
  3. 03Identifies unnecessary abstractions
  4. 04Suggests Rails-oriented alternatives
  5. 05Reviews performance and maintenance impact

Use cases

  1. 01Reviewing a Rails pull request
  2. 02Finding complexity in an existing app
  3. 03Assessing a proposed dependency or pattern

Install

Copy the command and run it in a terminal.

$npx skills add https://github.com/majesticlabs-dev/majestic-marketplace --skill dhh-code-reviewer

After installing, type/dhh-code-reviewerin Claude Code to confirm it loaded.

Details

From the project README

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-PatternRails Way
JWT tokensRails sessions
Separate API layersServer-side rendering + Hotwire
Redux-style stateRails' built-in patterns
MicroservicesMajestic monolith
GraphQLREST
Dependency injectionRails' elegant simplicity
.map(&:name).pluck(:name) - query directly
Logic-heavy partialsHelper methods
CSRF tokensSec-Fetch-Site headers (Rails 8+)
3. Complexity Analysis

Tear apart unnecessary abstractions:

Over-EngineeringSimple Solution
Service objectsModel methods
Presenters/decoratorsHelpers
Command/query separationActiveRecord
Event sourcing in CRUDStandard Rails
Hexagonal architectureRails conventions
Policy objects (Pundit)Authorization on User model
FactoryBotFixtures
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

  1. Start with what violates Rails philosophy most egregiously
  2. Be direct -- focus on actionable feedback
  3. Quote Rails doctrine when relevant
  4. Always suggest the Rails way as the alternative
  5. Champion simplicity and developer happiness
  6. 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
AvoidWhyAlternative
Devise500+ methods for simple auth~150 lines custom: Session model, authenticate_by, has_secure_password
OmniAuth (alone)Often overusedBuilt-in Rails auth + OmniAuth only for OAuth providers
Authorization
AvoidWhyAlternative
PunditSeparate policy classes add indirectionUser#can_administer?(resource) methods
CanCanCanMagic ability definitionsExplicit model methods
Background Jobs
AvoidWhyAlternative
SidekiqRequires RedisSolid Queue (database-backed)
ResqueRequires RedisSolid Queue
Caching
AvoidWhyAlternative
Redis cacheAnother dependencySolid Cache (database-backed)
MemcachedAnother dependencySolid Cache
WebSockets
AvoidWhyAlternative
Redis for Action CableAnother dependencySolid Cable (database-backed)
Testing
AvoidWhyAlternative
FactoryBotSlow, obscures dataFixtures - explicit, fast, version-controlled
RSpecDSL complexityMinitest - plain Ruby, readable
Architecture
AvoidWhyAlternative
Service objectsUnnecessary abstractionFat models with clear methods
Repository patternHides ActiveRecordUse ActiveRecord directly
CQRSOverengineeringStandard Rails MVC
Event sourcing (for CRUD)Complexity without benefitActiveRecord callbacks
Hexagonal/Clean architectureFights RailsRails conventions
JavaScript
AvoidWhyAlternative
React/Vue/AngularSPA complexityHotwire (Turbo + Stimulus)
Redux/VuexState management overheadRails sessions + Turbo Streams
GraphQLQuery complexityREST endpoints
JWT tokensStateless complexityRails sessions
CSS
AvoidWhyAlternative
Sass/LessNative CSS has caught upNative CSS (layers, nesting, custom properties)
CSS-in-JSWrong abstractionSeparate stylesheets
Infrastructure
AvoidWhyAlternative
KubernetesOperational complexitySingle container, Kamal
MicroservicesDistributed complexityMajestic monolith
PostgreSQL (for simple apps)Operational overheadSQLite (for single-tenant)
Database Design
AvoidWhyAlternative
Soft deletes (deleted_at)Pervasive null checks, bloated tablesHard deletes + event logs for audit
Auto-increment integer IDsEnumeration attacks, no client-side generationUUIDv7 (time-sortable, base36-encoded)
Boolean state columnsLoses who/when/whyState as records (e.g., Closure, Publication)
Views
AvoidWhyAlternative
Partials with mostly logicWrong abstraction levelHelper methods
Instance variables in helpersMagic dependenciesExplicit parameters
Complex cache key arraysFragile invalidationTouch 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.

Comments

Markdown supported