Key takeaways
- Correctness and security come before style
- AI written code gets the same scrutiny
- Small pull requests get better reviews
Reviewers work through these in order. Formatting and naming arguments come last, and a linter should settle most of them anyway.
Correctness
- Does the change do what the ticket and the specification ask?
- Are edge cases handled: empty lists, a zero quantity, a customer with no GSTIN?
- Are there tests, and do they check behaviour the user can see?
Security
- Is every input validated on the server?
- Are permissions checked on the server as well as in the UI?
- Are secrets kept out of code and out of logs?
- Is personal data kept to the minimum and protected?
Clarity
- Would someone new to the team follow this in six months?
- Are names clear and functions small?
- Is there a comment wherever the behaviour is not obvious from the code?
Performance
- Any database queries inside a loop?
- Any lists or uploads with no upper limit?
- Any work that could be cached or moved to a background job?
AI generated code
- Does it call APIs and library versions that actually exist? Assistants sometimes invent plausible ones.
- Does it follow the project's existing conventions, or bring in a new pattern of its own?
- Has the author run it and can they explain it? If not, it goes back.
Have a question about this guide? Write to us or ask Colony, our AI guide.