PR Code Review Guidelines
Posted by
admin
Wednesday April
8th
, 2026 4:07
p.m.
Drupal PR standards emphasize hard rules that block merges to ensure security, stability, and maintainability. These are enforced via tools like Coder (phpcs with Drupal standard) in CI pipelines.
Core Hard Rules (P1 Blockers)
These must pass before any PR approval, per Drupal.org contribution guidelines and core gate process.
No syntax errors or fatal issues: Run
composer lintorphpcs; zero tolerated errors, warnings, or deprecation notices for Drupal 11/PHP 8.2+.chromatichq+1100% Drupal coding standards compliance: 80-char line limit (except long declarations), braces on control structures (
if/else/for/switchalways braced, no inline), proper spacing around operators, no closing?>tags.pixelperfecthtml+2Automated tests required: PHPUnit coverage >90% on changed lines, Behat scenarios for behavior changes, no failures in kernel/functional tests.drupalize
Security must-haves: Parameterized queries only (no raw SQL),
\Drupal::entityTypeManager()->getAccessControlHandler()checks, CSRF tokens on forms, nountrustedCallbackscope pollution.salsa
Formatting Enforcements
Commit Standards
Single responsibility per commit with Conventional Commits format (e.g.,
fix(core): resolve N+1 in node loader).Signed-off-by footer required for core/contrib.
Extend P1 with these for Drupal-specific PRs. Use GitHub Actions with drupal/coder for auto-enforcement.droptica
Priority 1 (P1) Checks
Always block merges for these high-impact issues.
Ensure no syntax errors, fatal errors, or WSODs (White Screen of Death) in Drupal contexts—run
drush lintand test in a staging environment mimicking production caching.jamestarletonVerify automated tests pass: Behat scenarios cover new/changed behavior, PHPUnit units have >80% coverage on modified lines, and no regressions in CI.jamestarleton
Confirm security basics: No raw SQL without placeholders, proper access checks via hooks or services, and sanitized user inputs everywhere.jamestarleton
Check commit hygiene: Descriptive messages following Conventional Commits (e.g., "feat: add user profile caching"), single purpose per commit, and signed off.linkedin
Priority 2 (P2) Improvements
Request changes but allow conditional merges with follow-ups.
Code Readability and Standards: Follow Drupal coding standards (run
phpcs --standard=Drupal), use meaningful variable names, and limit functions to <50 lines. Prefer early returns over deep nesting.jamestarletonPerformance: Add cache tags/contexts for render arrays, avoid N+1 queries (use Entity Query or eager loading), and invalidate caches properly via Drush post-deploy.jamestarleton
Configuration Management: All changes via config export/import; no hardcoded values. Test config sync across environments.jamestarleton
CSS Best Practices
Discourage !important entirely—it's abused for quick fixes but leads to specificity wars and unmaintainable stylesheets.j11y+1
Use stronger, more specific selectors instead (e.g., .container .button.primary over .button). Learn CSS specificity rules: IDs (100), classes/attributes (10), elements (1)—hover selectors in Chrome DevTools for values.world.hey+1
(assuming a specifishity.com graphic; visualize via https://specifishity.com/ for training).j11y
Advanced Refactoring (Move to Separate Articles)
These warrant dedicated guides given their depth.
Static Methods: Refactor poor usage (e.g., god objects) into services with proper DI via container injection, not procedural globals.linkedin
Dependency Injection Habits: Always type-hint interfaces in constructors; avoid service locators. Example:
\Drupal::service('cache.factory')→ injectedCacheBackendInterface.jamestarleton
Quick Checklist Table