Code review

Code review is the practice of reading changes before they land in the main branch. At the tool level it is comments in a Pull Request (see pull-request). At the culture level it is a distinct skill that affects team velocity more than the choice of framework.

Why it matters

Primary functions, in descending order:

1. Knowledge sharing. A year from now the reviewer still knows what happens in this piece of code. Without review only the author does. The team becomes more resilient to departures and vacations.
2. Keeping standards alive. Every review is a conversation about "how we write code here." The standard stays current.
3. Finding problems. Logic errors, uncovered edge cases, performance issues, security gaps. The author has tunnel vision; the reviewer sees with fresh eyes.
4. Guarding against regressions. A senior reviewer catches "this change will break another system."
5. Mentorship. Feedback is the primary growth mechanism for a new team member.

"Catching bugs" is fifth on the list. Most bugs are found by tests; review catches what tests do not check (architectural decisions, readability, implicit assumptions).

Good practices for the author

• Small PRs. Under 300 lines; ideally under 100. The reviewer reads faster and finds more.
• PR description. Why, what, how to verify. Without a description the reviewer spends the first ten minutes reconstructing context.
• Screenshots for UI changes. Saves the reviewer a local run.
• Self-review before requesting review. Read your own diff and fix the obvious. No one else should find your console.log or TODO.
• Do not take comments personally. Review is about the code, not the author. If a comment sounds harsh it is usually the reviewer's habit, not malice. Ask "what do you suggest?" instead of defending your approach.

Good practices for the reviewer

• Respond quickly. A PR with no response for more than 24 hours means the author loses context. A few hours is the ideal.
• Explain, do not command. Not "fix this" but "there may be a problem X here: what do you think?" Especially with junior contributors.
• Distinguish the level of each comment. Use prefixes:
• nit: or (non-blocking): minor, can be ignored.
• suggestion:: an idea, not required.
• must: or (blocking): needs to be fixed before merge.
• question:: just a question, no criticism implied.
• A comment needs a reason. "Could be better" is bad. "A null reference is possible here, see line X in auth.py" is good.
• Approve when it is ready. Do not "approve with comments." If there is a blocker, request changes. If there is no blocker, approve.

Comment structure (Conventional Comments standard)

praise: Good use of composition here, clean.

nit: Better to rename `data` to `userResponse` for clarity.

suggestion: Could be extracted into a utility, used in three places.

question: Why is there a try/catch with no error handling?

issue (blocking): This code swallows the error. Either rethrow it

or log it.

The prefix immediately signals tone and urgency. It speeds up discussion considerably.

What to look for

A checklist for reviewers:

• Clarity. Will another developer read this and understand it a year from now?
• Correctness. Does it solve what the PR description says?
• Edge cases. What happens with null, an empty array, very large data, a race condition?
• Tests. Do they cover the change? Do they test behavior, not implementation?
• Security. Is user input sanitized? Are access controls checked?
• Performance. Did an O(n²) creep in where O(n) is expected?
• Consistency. Does the style match the rest of the codebase?

Do not try to find everything in one pass. Start with understanding (do I know what is happening?), then correctness, then details.

Pitfalls

• Bikeshedding. Endless debate over a button color or variable name. Agree on a time-box or let a linter rule decide.
• Rubber-stamp approval. Common on large PRs, but it means review is not working. The fix is requiring smaller PRs.
• Hierarchy mismatch. A senior reviews a junior too harshly; a junior reviews a senior too timidly. Agree that in review there is no hierarchy and both people are responsible for quality.
• Ego review. "I would have written this differently." If the code solves the problem and reads clearly, it is a legitimate approach. That is not a reason to demand a rewrite.