Moko Consulting

Open-source software for Joomla, Gitea, and web platforms. Home of MokoSuite, MokoGitea, and MokoCLI.

Tennessee
standards/code-review.-

Code Review

Code review checklist, approval process, and expectations.

Review Requirements

  • Every PR must be reviewed before merging to main
  • One maintainer approval required
  • CI must pass (lint, phpcs, phpstan, tests)
  • Target turnaround: 48 hours for initial review

Review Checklist

Security

  • No SQL injection — all queries use prepared statements
  • No XSS — all output escaped with htmlspecialchars() or Joomla's Text class
  • CSRF protection — Session::checkToken() on all form submissions
  • Input validation — user input validated and sanitized via InputFilter
  • No hardcoded credentials, tokens, or API keys
  • File uploads validated (type, size, extension whitelist)
  • Authorization checks — ACL verified before data access

Performance

  • No N+1 queries — use JOINs or batch loading
  • Database queries indexed for common access patterns
  • No unnecessary loops or redundant processing
  • Assets loaded via Web Asset Manager (enables caching)
  • Large datasets paginated

Code Quality

  • Follows naming conventions (see coding standards)
  • No dead code, commented-out blocks, or debug statements
  • Functions/methods have single responsibility
  • Error handling is explicit (no silent failures)
  • Type declarations on all parameters and returns (PHP)

Backwards Compatibility

  • Database schema changes have update SQL scripts
  • API changes are additive (no breaking removals)
  • Configuration changes have migration path
  • Language strings added for new UI text

Testing

  • New features have tests
  • Bug fixes include regression test
  • Tests pass locally and in CI

Documentation

  • PHPDoc on new public methods
  • CHANGELOG.md updated
  • README updated if user-facing behavior changed

Review Comments

  • Be specific — reference line numbers and suggest alternatives
  • Use "suggestion" blocks for code changes
  • Distinguish "must fix" from "nice to have" (prefix with nit: for minor suggestions)
  • Ask questions rather than making assumptions about intent
  • Approve when satisfied — don't block on nitpicks

Merge Process

  1. PR approved by maintainer
  2. CI passes
  3. Squash merge for feature branches
  4. Delete branch after merge
  5. Verify deployment if applicable