What I look at
- File structure and maintainability: will the next person know where this belongs and how to change it?
- File size, as a signal to check whether a file has taken on too many responsibilities. It isn’t a universal limit.
- Semantics: correct HTML elements, accurate names, and code that says what it means.
- Unnecessary complexity, missing abstractions, and dead code.
- Comments that explain why something is done, rather than repeating what the code already says.
- Performance problems that real users would feel.
- Security concerns, especially around input, authentication, and data exposure.
Required changes versus preferences
I separate what must change before merging from what I would personally do differently. A bug, a security issue, or a broken agreed standard is a required change. A naming choice or a different but valid approach is a suggestion, and I label it that way so the author can decide.
When we disagree
I explain the trade-off behind a request rather than just stating a rule. If we still disagree, we settle it with established standards or a senior reviewer, not by whoever holds out longest.
My approach is not the team’s standard
Everything above is how I review. It is separate from the standards a team has agreed to. In my current role, the standards the team adopted for ongoing work are:
- A modular, feature-based file structure.
- Reusable components instead of repeated variants.
- Complete form validation.
- Separation of concerns so files keep one responsibility.
- A shared notification system for success and error feedback.
- Consistent, brand-aligned UI and interaction behaviour.
- A README that matches the actual project.