Code Review Phrases
A quick-reference phrasebook — 30 ready-to-use phrases you can look up and copy for giving and receiving code review feedback: approvals, suggestions, blocking issues, and PR descriptions. Want interactive practice instead? Try the Code Review Language exercises.
- Use Nit: to mark non-blocking minor suggestions
- Use Blocking: for things that must change before merge
- Phrase suggestions as questions or options — not commands
- Be explicit about approval: "Nothing blocking" means different things to different people
Approving & Positive Feedback
- This looks good to me. Approved! ✓Clear, concise approval
- LGTM! (Looks Good To Me)Common abbreviation — informal approval
- Clean implementation — no notes.Praise without padding
- I like the approach here — clear and easy to follow.Specific positive on readability
- Nice use of X here — this will make future changes easier.Acknowledge forward-thinking design
- Left a few minor comments — nothing blocking.Signal overall approval with small nits
Suggesting Improvements
- Nit: could rename this to [name] for clarity."Nit" = non-blocking minor suggestion
- Could you consider extracting this into a separate function?Soft suggestion — question form
- I'd suggest…Assertive but non-demanding
- What do you think about…?Invite the author's opinion first
- This might be cleaner as…"Might" lowers the pressure
- Optional: you could also do X, but this works too.Mark a suggestion as truly optional
- Not a blocker, but worth noting for future refactors.Defer non-urgent feedback explicitly
Requesting Required Changes
- This needs a test for the edge case where…Direct request — no softening needed for tests
- Can you add error handling for the case where X is null?Question form softens a requirement
- Blocking: this will cause a bug when [condition].Explicitly flag showstoppers
- I'd want to see a test before approving this.Clear condition for approval
- This might cause a race condition if X and Y happen concurrently.Flag a potential bug diplomatically
- The logic here doesn't handle [edge case] — can you add that?Point to the gap, ask for the fix
Asking Questions & Clarifications
- Can you explain the reasoning behind this approach?Genuine question — not a challenge
- Why X instead of Y here?Direct — fine if you're curious, not critical
- Is this intentional, or a leftover from earlier?Check before flagging a bug
- Am I missing something, or does this not handle [case]?"Am I missing something" — humble framing
- Out of curiosity — is there a reason to prefer X over Y here?Learning question, not a blocker
- Happy to discuss in a call if this is complex to explain in comments.Offer async-to-sync escalation
Writing a Strong PR Description
- This PR…Always start with what the PR does
- Fixes #[issue number] — [brief description]Link to the issue
- How to test: 1) Run X 2) Check Y 3) Expect ZNumbered test steps are easiest to follow
- No breaking changes.Explicitly call this out
- Breaking change: [what changed] — migration steps below.Lead with the impact
- Screenshots / recordings below.For UI changes
- Known limitation: [X]. Tracked in #[issue number].Honest about constraints
Frequently Asked Questions
What does 'Refactor' mean in a code review context? I'm seeing this term often, but don't understand if it means just making small changes.
'Refactor' refers to restructuring existing computer code—altering the internal structure without changing its external behavior. It's about improving readability, maintainability, or performance by simplifying logic, removing duplication, or applying design patterns. A refactor is a deliberate change aimed at making the code cleaner and easier to understand for future development.
I keep seeing 'DRY' mentioned. What does DRY stand for in a code review, and why is it important?
'DRY' stands for Don't Repeat Yourself. It's a core principle emphasizing avoiding redundancy in your codebase by extracting common logic into reusable functions or modules. This reduces the risk of inconsistencies and simplifies maintenance – if you need to change something, you only modify it in one place.
What's the difference between 'Code Smells' and a 'Bug'? I'm confused when reviewers point out 'code smells'.
'Code Smells' are patterns or characteristics in code that indicate potential problems, like excessive complexity or duplicated logic – they aren't necessarily errors. A 'bug', on the other hand, is a demonstrable flaw where the code doesn't behave as intended. Reviewers highlight smells to suggest areas needing attention before they become bugs.
Can someone say 'This function lacks context'? What does that actually mean during a review?
'Lacking context' usually indicates the function's purpose isn't immediately clear from its code. The reviewer is suggesting the function needs clearer documentation, better naming conventions, or perhaps restructuring to explicitly state its role within the broader system architecture.
I received feedback about 'Duplicated Logic'. What specific actions should I take to address this?
'Duplicated Logic' refers to identical code blocks appearing in multiple places, often leading to maintenance issues. You should extract the common logic into a reusable function or method and call that from both original locations; this eliminates redundancy and ensures consistency.
What is 'Technical Debt' and how does it relate to code reviews?
'Technical Debt' represents compromises made in design, implementation, or documentation to meet short-term deadlines. Code reviews can help identify and mitigate technical debt by highlighting areas where shortcuts were taken, allowing for a planned approach to address them later.
I've been asked to 'Simplify this logic'. What does that imply during the review process?
'Simplify' suggests the code is overly complex or convoluted. The reviewer likely wants you to reduce unnecessary steps, improve readability by using simpler algorithms or data structures, and ultimately make the code easier to understand and maintain.
What's the purpose of saying 'This variable could be named better'? Give me an example.
A poorly named variable can hinder understanding. For instance, a variable called 'x' without context is ambiguous. Rename it to something descriptive like 'customerOrderTotal' or 'userAuthenticationToken' so its purpose within the code becomes immediately clear.
The reviewer suggested 'Increase test coverage'. What does that actually mean in practice?
'Increasing test coverage' means adding more tests to your codebase, particularly focusing on edge cases and areas with complex logic. This ensures that changes don't introduce regressions by verifying functionality through a wider range of scenarios.
I saw the phrase 'Consider using an abstraction layer'. What does this suggest during review?
'Considering an abstraction layer' indicates the code is tightly coupled and complex. It suggests introducing an intermediary component or interface to hide implementation details, promoting modularity, reducing dependencies, and making the code more flexible for future changes.