I've done over 100 code reviews in the past year. Some for clients. Some for open source projects. Some for friends who asked me to "take a quick look" (it's never quick).
After all those reviews, I noticed patterns. The same mistakes keep showing up. The same things keep impressing me. And the biggest lesson? The best code isn't clever. It's boring.
Let me break it down.
The #1 Thing Seniors Look For (It's Not What You Think)
Every junior developer thinks code review is about finding bugs. It's not.
Sure, we catch bugs. But the real purpose of code review is answering one question: "Can someone else understand and maintain this code six months from now?"
That's it. If the answer is yes, the code passes. If the answer is no, it doesn't matter how technically brilliant it is.
I reviewed a pull request once that used five design patterns, three custom operators, and a recursive generic type. It was technically impressive. It was also completely unreadable. I asked the developer to rewrite it in plain, simple code. They were offended. But six months later, when someone else had to modify that feature, they thanked me.
The 7 Most Common Mistakes I See
Mistake 1: Giant Pull Requests
This is the number one issue. Someone works on a feature for two weeks, then opens a PR with 47 changed files and 2,000+ lines of code.
Nobody is going to review that properly. They'll skim it, approve it, and pray nothing breaks.
The fix: Keep PRs under 400 lines of actual code changes. If your feature is bigger, break it into smaller PRs. Ship a foundation PR first, then build on top of it.
I've started rejecting any PR over 500 lines unless there's a very good reason. It sounds strict, but it caught three major bugs that would have slipped through in larger reviews.
Mistake 2: Meaningless Variable Names
# Bad
d = get_data()
r = process(d)
x = transform(r)
# Good
user_profiles = fetch_user_profiles()
filtered_profiles = remove_inactive(user_profiles)
profile_summaries = generate_summaries(filtered_profiles)
I see this constantly. Single-letter variables, abbreviations nobody understands, names that describe the type instead of the purpose.
Your variable name is documentation. Treat it that way.
Mistake 3: No Error Handling
// Bad: pretending errors don't exist
const data = await fetch('/api/users').then(r => r.json());
// Good: handling the real world
try {
const response = await fetch('/api/users');
if (!response.ok) {
throw new Error(`Failed to fetch users: ${response.status}`);
}
const data = await response.json();
} catch (error) {
logger.error('User fetch failed', { error });
showErrorToast('Could not load users. Please try again.');
}
Happy path code is easy. Production code handles the unhappy path. Every network call can fail. Every user input can be wrong. Every file might not exist.
If I see a PR with zero error handling, it goes straight back for revision.
Mistake 4: Comments That Explain "What" Instead of "Why"
# Bad: tells me what the code does (I can read the code)
# Increment counter by 1
counter += 1
# Good: tells me WHY we're doing this
# Users get 3 free attempts before requiring payment
# Reset counter after successful verification
counter += 1
Your code should be readable enough to explain WHAT it does. Comments should explain WHY it does it. The business logic, the edge case it handles, the reason for a weird workaround.
The best comment I ever read was: "This looks wrong but it's correct. The API returns dates in UTC-5 even when you request UTC. See ticket PROJ-1234."
That comment saved someone hours of debugging.
Mistake 5: Reinventing the Wheel
I reviewed a PR where someone wrote a custom 200-line date formatting utility. The project already had date-fns installed.
Before writing any utility function, check if:
- Your language/framework has a built-in solution
- Your project already has a library that does this
- Someone on your team already wrote this
I'd estimate 30% of the custom utility code I review already exists somewhere in the project or its dependencies.
Mistake 6: Mixing Concerns in One Function
# Bad: this function does everything
def process_order(order):
validate_items(order.items)
calculate_tax(order)
apply_discount(order)
charge_payment(order)
send_confirmation_email(order)
update_inventory(order)
log_analytics(order)
# Good: each function does one thing
def process_order(order):
validated_order = validate_order(order)
priced_order = calculate_pricing(validated_order)
payment_result = process_payment(priced_order)
finalize_order(priced_order, payment_result)
Every function should do one thing. If you can't describe what a function does without using the word "and," it's doing too much.
Mistake 7: No Tests for Edge Cases
Most developers test the happy path. Order goes through, user logs in, data loads. Great.
But what about:
- Empty arrays?
- Null values?
- Network timeouts?
- User submits the form twice?
- Input has 10,000 characters?
The bugs that make it to production are almost always edge cases. Test them.
5 Things That Make Me Approve a PR Instantly
1. Small, Focused Changes
One PR, one purpose. "Add user search" or "Fix date parsing bug." Not "Add search, refactor database layer, update styles, and fix that thing from last week."
2. Self-Documenting Code
When I read the code and don't need to ask any questions, that's good code. Clear function names, obvious variable names, logical file structure.
3. Tests That Cover Edge Cases
When I see tests for empty inputs, error states, and boundary conditions, I know the developer thought about production, not just the demo.
4. Clean Git History
Atomic commits with clear messages. Not "fix stuff" or "WIP" or "asdfgh." Each commit should tell a story.
feat: add user search endpoint
test: add integration tests for user search
docs: update API documentation for search endpoint
5. They Reviewed Their Own Code First
You'd be amazed how many issues you can catch by looking at your own diff before submitting. I always review my own PRs before asking anyone else to. It catches typos, leftover debug logs, and "oh wait, I forgot to handle that case."
How to Write Code That's Easy to Review
Here's my checklist before opening a PR:
Before writing:
- [ ] Is the task clear? Do I understand what "done" looks like?
- [ ] Have I checked if similar code exists in the project?
While writing:
- [ ] Are my functions short and focused?
- [ ] Are my variable names descriptive?
- [ ] Am I handling errors properly?
- [ ] Am I writing tests as I go?
Before submitting:
- [ ] Is the PR under 400 lines?
- [ ] Did I review my own diff?
- [ ] Did I remove debug logs and commented-out code?
- [ ] Does the PR description explain WHAT and WHY?
- [ ] Do all tests pass?
The Meta-Lesson
After 100 reviews, the biggest thing I've learned is that coding is communication. You're not writing instructions for a computer. You're writing a message to the next developer who'll read your code.
That developer might be a new hire. They might be sleep-deprived. They might be you in six months. Write code for them.
Every clever one-liner you write is a support ticket waiting to happen. Every well-named function is a question that never gets asked. Every good test is a bug that never reaches production.
The developers I respect most don't write the smartest code. They write the clearest code. And when you read their PRs, you think, "Oh, that makes sense." That's the highest compliment in software engineering.
The Takeaway
Code review isn't about gatekeeping. It's about raising the bar for everyone, including yourself. The best way to get better at writing code is to read a lot of other people's code and have other people read yours.
If you're a junior developer, don't fear code reviews. Request them. Ask for brutal honesty. The feedback you get from a good code reviewer in 30 minutes can save you months of learning the hard way.
And if you're the reviewer, remember: your job isn't to prove you're smarter than the author. Your job is to help them ship better code.
If you found this useful, I share more stuff like this on Telegram and sell developer toolkits on Boosty.
Top comments (0)