The Problem
A growing engineering team working across multiple client codebases without enforced standards produces inconsistency at scale. Different engineers scaffold the same Magento patterns differently — varying module structures, inconsistent DI configuration approaches, different naming conventions, varying levels of test coverage. None of these inconsistencies are obviously wrong in isolation. Collectively, they compound into maintenance overhead.
The secondary effect is on team dynamics: code review becomes subjective when there’s no defined standard to refer to. Review comments that are preferences rather than violations are harder to navigate and create friction without improving outcomes.
Standards documented in a wiki are read once. Standards enforced by CI are followed on every commit.
The Standard
Module Structure
A defined module directory layout, naming convention, and DI configuration pattern — applied to all new Magento modules regardless of client project. Key structural decisions:
- Interface-based design: public API defined as interfaces, implementation separate
- Service layer separation: model classes for ORM, dedicated service classes for business logic
- Test directory structure mirrors source structure — a missing test file is visible in the PR diff
New modules are scaffolded from a template that produces the correct directory structure, stub files, and registration.php, etc/module.xml, and composer.json with the correct values. Engineers fill in the implementation, not the structure.
Code Style
PHPCS configured with a custom ruleset:
- PSR-12 as the base
- Magento-specific additions: template security rules (escaped output requirements), DI type hint requirements, and naming convention enforcement
- Zero tolerance for new violations — no “we’ll fix it in the next sprint”
PHPStan at maximum strictness level. Type coverage enforced. Nullability violations caught. Unused imports flagged. The strictness level was increased incrementally over time as the codebase was brought into compliance — raising it to maximum on a legacy codebase in one step is impractical.
Both tools run in CI as blocking gates. The enforcement is not “engineers should run PHPCS locally” — it’s “CI fails if PHPCS fails.”
Code Review Process
Standard changes: Two-engineer sign-off required.
Security-sensitive paths (checkout, payment flows, authentication, session handling): Three-engineer sign-off required. The additional reviewer brings a security-specific lens.
Review structure:
- Architecture: does the change fit the established patterns? is there a simpler approach?
- Security: any input handling, output escaping, or access control concerns?
- Performance: any N+1 queries, missing cache invalidation, or unindexed lookups?
- Tests: are the changed code paths covered?
Open review comments must be resolved, not dismissed. “LGTM, but…” followed by approval is not a resolved comment.
PR descriptions: Required to include what changed, why, and what to test. Not every PR needs a novel — a one-paragraph description covering these three points is sufficient. Missing or placeholder descriptions are returned with a request to fill them in before approval.
What’s Not in the Standard
The standard doesn’t cover: IDE configuration, personal development workflow, preferred debugging tools, or opinions on spacing within function bodies. These generate more heat than light in code review and don’t affect the output. The standard covers the things that make codebases inconsistent or unsafe — not the things that are preferences.
Rollout
Rolling the standard out to existing codebases required a different approach than applying it to new development. Legacy code that fails PHPCS rules couldn’t be fixed all at once without introducing risk. The approach:
- Static analysis baseline established: existing violations recorded, new violations blocked
- PHPCS ratchet: no new violations from the rollout date; existing violations addressed module-by-module as code is touched
- Module template adopted for all new modules immediately
- Review process formalised and documented; engineers trained through paired reviews
The process was complete (all active codebases fully compliant) within two quarters.
Impact
The observable effects of the standard are partly in what’s absent: fewer review cycles about style preferences, fewer onboarding conversations about “how we do things here,” fewer production incidents from inconsistent patterns in security-sensitive code paths.
The onboarding effect is measurable: new engineers contribute to existing codebases faster because the patterns are consistent. The pattern a new engineer sees in one module is the same pattern they see in every other module.