Skip to content

Instantly share code, notes, and snippets.

@ajaxray
Created January 29, 2026 19:37
Show Gist options
  • Select an option

  • Save ajaxray/4753288f9342481b8745fe44c22d2505 to your computer and use it in GitHub Desktop.

Select an option

Save ajaxray/4753288f9342481b8745fe44c22d2505 to your computer and use it in GitHub Desktop.
PR Review Red Flags for PHP / Laravel Application

PR Review Red Flags (PHP / Laravel Application)

1. Model::create([...]) instead of new Model()

Why it matters: create() bypasses explicit assignment and hides mass assignment logic, making code less transparent about which fields are being set. Using new Model() followed by explicit propertyassignment makes dependencies and validation clearer, improving testability and reducing accidental mass assignment vulnerabilities.

2. Business logic in controllers

Why it matters: Controllers should orchestrate, not implement business rules. Logic in controllers couples your business rules to HTTP concerns, making them untestable without mocking the entire request/response cycle. Extract to Services/Actions for reusability across jobs, commands, and queues.

3. Auth::user() in models

Why it matters: Models should be authentication-agnostic; calling Auth::user() couples them to the framework and HTTP context. Pass the authenticated user explicitly as a parameter, allowing models to work in queued jobs, commands, and other non-HTTP contexts.

4. Debug code in PR

Why it matters: Debug statements (dd(), var_dump(), console.log()) left in production code break functionality or expose sensitive information. These are caught automatically by CI linters; their presence suggests inadequate pre-submission review and increases risk of production incidents.

5. setTimeout for API calls

Why it matters: Polling with timeouts is unreliable, wastes resources, and creates race conditions. Use proper async/await patterns, webhooks, or event-driven architectures instead. Timeouts are a symptom of architecture that hasn't properly addressed asynchronous concerns.

6. Arrays passed to Actions instead of DTOs

Why it matters: Bare arrays are untyped and lack IDE autocomplete; DTOs provide type hints, self-documentation, and a clear contract. This catches bugs early, improves developer experience, and makes refactoring safer.

7. Direct JSON returns instead of Resources

Why it matters: Resources (in Laravel) provide a consistent response format, transformation layer, and versioning strategy. Direct json_encode() returns scatter data transformation logic across your codebase and make API changes fragile.

8. Missing table prefix in migrations

Why it matters: Without a prefix, table names risk conflicts in shared databases or when integrating multiple systems. Prefixes provide namespace isolation and are essential for multi-tenant or modular architectures.

9. ->value on enums in PHP code

Why it matters: Calling ->value breaks type safety; use the enum directly. Type system benefits (autocompletion, static analysis) are lost. Store enums natively in your database and pass enum objects (with proper casting), not strings, through your code.

10. Manual event listener registration without comment

Why it matters: Silent event registration (no comment explaining why the listener exists) makes code harder to trace and maintain. The binding is implicit; a comment clarifies the intention and prevents accidental removal during refactoring.

11. Traits in Traits/ folder

Why it matters: Traits are structural mixins, not standalone constructs. Naming them after their behavior (e.g., HasTimestamps, LogsActivity) and grouping by domain (e.g., Models/Concerns/) improves discoverability and makes dependencies explicit.

12. Method names not matching return types

Why it matters: A method named getUser() should return a User, not a Collection. Mismatches break the principle of least surprise, confuse developers, and hide type expectations. Rename to getUsers() or change the return type.

13. N+1 Query Problems (Missing with() / load() Clauses)

Why it matters: Lazy loading in loops causes exponential database queries, destroying performance. Always eagerly load relationships with with() to make query patterns explicit and measurable. This is easy to spot in PR code and critical for scalability.

14. Exception Handling Without Logging

Why it matters: Silent catch blocks hide errors; exceptions must be logged or re-thrown. Swallowing exceptions makes debugging in production nearly impossible and masks systemic issues that should trigger alerts.

15. Hardcoded Configuration Values

Why it matters: Magic strings/numbers scattered in code are unmaintainable and prevent environment-specific configuration. All configurable values belong in .env or config files, not hardcoded in controllers or services.

16. Missing Input Validation in Actions/Commands

Why it matters: Skipping request validation or DTO validation allows invalid data to propagate through your system, causing subtle bugs. Validation should be explicit and fail fast, ideally at the entry point.

17. Unnecessary Database Queries in Loops

Why it matters: Any database operation inside a loop (even within a relationship callback) is a red flag. Batch operations with whereIn(), chunking, or query aggregation should be the default approach.

18. Missing Type Hints on Public Methods

Why it matters: Untyped public methods force callers to guess input/output types and prevent IDE assistance. Type hints are free documentation and catch mistakes at development time, not production time.

19. Shared State in Static Properties

Why it matters: Static properties persist across requests in long-running processes (workers, async handlers), causing data leaks between requests. Use instance properties or dependency injection instead.

20. Testing Non-Deterministic Behavior Without Mocks

Why it matters: Tests that depend on current time, random numbers, or external APIs without mocking are flaky and unreliable. Mock dependencies; if you can't mock something, it signals a design issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment