Skip to content
davthecoder
What I Look for During an Android Code Review
tech

What I Look for During an Android Code Review

By David Cruz Anaya, Independent Senior Mobile Engineer (Android, Kotlin Multiplatform, Rust)

Updated 15 July 2026
AndroidCode ReviewKotlinMobile DevelopmentEngineering Practices
Share:

A reviewer who tries to read a thousand-line pull request in one sitting will miss things. In the Cisco study behind SmartBear’s peer-review guidance, defect-finding fell off sharply once a single review passed roughly 400 lines of code (SmartBear, Best Practices for Peer Code Review). That one finding shapes how I approach an Android code review.

Over twelve years, at Sky, BritBox, BetMGM and my own clients, the fast rubber-stamps never caught the real problems. So this is the checklist I run in my head when I open an Android pull request. It starts with correctness and ends with tone, and the order matters more than the list.

Key Takeaways

  • Reviews of under about 400 lines over 60 to 90 minutes find 70 to 90 percent of defects (SmartBear).
  • I review in priority order: correctness, architecture, runtime behaviour, tests, then style. Style is last on purpose.
  • After adopting Kotlin, Google Home cut its top crash cause, NullPointerExceptions, by 33 percent (Google).
  • A small, well-described pull request that comes back within a day beats a perfect review that arrives three days late.

Why bother with an Android code review at all?

Reviews are sold as bug-catching, but their bigger return is shared understanding. When Microsoft researchers studied modern code review, they found that finding defects was the stated motivation, yet in practice reviews delivered more value through knowledge transfer, team awareness and better solutions than through defect-finding alone (Microsoft Research, Expectations, Outcomes, and Challenges of Modern Code Review, 2013).

That framing changes how I review. I am not there to prove I am smarter than the author. I am there to make sure the change is correct, to make sure a second person understands it, and to leave the author a little better than I found them.

Two developers reviewing code together at a desk with laptops and a monitor.

From experience: the single most useful habit I have is reviewing in priority order and saying so. Correctness and architecture first, naming and formatting last. When you lead with a nit about a variable name, the author fixes the name and stops reading, and the real bug three files down never gets discussed.

If you want that same review discipline applied to your own repository, that is one of the things I do in one-to-one code and architecture reviews.

Does the code actually do what the pull request says?

Correctness comes first, and on Android that starts with null-safety, threading and lifecycle. Kotlin’s type system pays off here in a measurable way. In 2020, the Google Home team reported a 33 percent drop in NullPointerException crashes after moving to Kotlin, and NPEs had been their number one crash cause (Google, Google Home reduces its #1 crash cause by 33%, 2020). Google has also said apps in the top 1,000 on Google Play that use Kotlin see 20 percent fewer crashes per user.

Kotlin’s crash-reduction effectWhat null-safety buys youCrash reduction reported after adopting Kotlin33%20%fewer NPE crashes(Google Home)fewer crashes/user(top 1,000 apps)
Source: Google, Android Developers Blog and developer.android.com, 2020.

The compiler will not save you from everything, though. So I read the change against the ticket and ask blunt questions. Does it handle the empty state and the error state, not just the happy path? Is any blocking work happening on the main thread? Are coroutines scoped to something that gets cancelled, or are they leaking past the screen that launched them? Kotlin makes a lot of this readable, but nullability, cancellation and platform types still hide bugs that only a human notices.

For the coroutine side of that, I have written up how Job and SupervisorJob change cancellation behaviour, and for the language features I lean on most, what inline, crossinline, noinline and reified actually do. Romain Guy’s droidcon London 2024 talk, Optimizing Kotlin Code in Practice, is also a great tour of idiomatic versus costly Kotlin, which is exactly the lens I bring to this part of a review.

Is the architecture heading the right way?

Architecture review is about direction, not perfection. Google’s study of its own review process, across nine million reviewed changes, found that healthy review culture favours small, single-reviewer changes rather than big design debates bolted onto a diff (Google Research, Modern Code Review: A Case Study at Google, 2018). By the time code is in a pull request, the architecture is mostly decided, so my job is to catch the change that quietly points the codebase in the wrong direction.

I look at where the code lives and what it now depends on. Does this belong in the ViewModel, or has business logic leaked into the Composable or the Activity? Did a data-layer class just start importing something from the UI layer? Is the change adding a new pattern when three similar ones already exist? I also watch for the opposite problem, which is over-engineering: a generic abstraction with a single caller is a liability, not a kindness.

What I actually do: I ask “will the next feature be easier or harder because of this change?” If the answer is harder, that is an architecture comment, and it outranks every style nit in the diff.

When a team is deciding what to share and what to keep native, that same thinking scales up. I go deeper on it in my blueprint for Kotlin Multiplatform clean architecture.

How will this behave on a real device?

Android code has to survive a hostile runtime, so I read every diff as if it will run on a mid-range phone with a flaky connection and an aggressive battery optimiser. Does the change do disk or network work on the main thread? Does it hold a Context, View or Activity reference somewhere it can outlive the screen and leak? What happens on a configuration change, a process death, or a fast rotation while the network call is in flight?

Developer coding on a laptop while holding an Android smartphone.

Jetpack Compose adds its own runtime questions. Is state hoisted correctly, or is a Composable reading and writing the same state in a way that triggers extra recomposition? Are the types passed into Composables stable, so Compose can skip work it does not need to redo? Is a long list backed by keys and a lazy layout, or will it rebuild the world on every change? None of this shows up in a screenshot, which is why it belongs in the review rather than in QA. For the Compose-specific decisions I still reach for, see ConstraintLayout in Jetpack Compose.

Is it tested, and will it age well?

I check that the change is tested and that the code will not rot, because review is one of the few moments where maintainability is cheap to defend. GitClear’s 2025 analysis of 211 million changed lines found refactoring fell from about 25 percent of changed lines in 2021 to under 10 percent by 2024, while copy-pasted lines rose from 8.3 to 12.3 percent (GitClear, AI Copilot Code Quality: 2025 Data, 2025). Duplication is creeping back into codebases, and review is where you catch it.

On tests, I am not chasing a coverage number. I want the test to fail if the behaviour breaks, to read like documentation of intent, and to avoid asserting on implementation details that will change next week. A test that mocks everything and asserts that the mocks were called proves nothing. I would rather see three tests that pin the real edge cases than thirty that pad a coverage report.

On duplication, I flag the second copy, not the third. The first time code is copied it is forgivable, the second time it is a pattern, and by the third the cost of pulling it together has already been paid several times over.

Is this pull request even reviewable?

A change I cannot hold in my head is a change I cannot review well, so size and description are not politeness, they are correctness. The Cisco study behind SmartBear’s guidance is blunt about the limits: keep a single review under roughly 400 lines, inspect no faster than about 500 lines an hour, and do not review for more than 60 minutes at a stretch. Stay inside those and a review finds 70 to 90 percent of the defects present (SmartBear, Best Practices for Peer Code Review).

The guardrails for an effective reviewWhere a review stays effectiveStay inside these and you catch 70-90% of defects400lines / review500lines / hour60minutes / sessionUpper limits, not targets. Smaller is better.
Source: SmartBear, Best Practices for Peer Code Review (Cisco Systems study).

If the diff is bigger than that, I say so before I review, and I ask for it to be split. I also read the description first: what changed, why, and how it was tested. A pull request with no description is a request to reverse-engineer someone else’s intent, and that is where reviews go quiet and defects slip through. For the author’s side of this, Best Practices for Pull Requests is a solid, practical watch.

How quickly should a review come back?

Fast is a feature, and a slow review is a tax the whole team pays. Google’s own engineering guidance puts the ceiling at one business day to respond to a review request, and notes that responding quickly matters more than finishing the whole review at once (Google, Speed of Code Reviews). Elite teams keep this tight: LinearB’s benchmarks across more than eight million pull requests put top-tier pickup time under seven hours, review time under six, and full cycle time under 25 hours (LinearB, Engineering Benchmarks, 2024 to 2026).

Elite pull request benchmarksElite pull request timingsTop-tier teams, P75 across 8M+ pull requestsPickup timeunder 7hReview timeunder 6hFull cycle timeunder 25h
Source: LinearB, Software Engineering Benchmarks, 2024 to 2026.

This is why I turn reviews around quickly even when I cannot do a full pass. A two-line comment that unblocks someone in an hour is worth more than a thorough review that lands tomorrow afternoon.

How I give the feedback

How I say something decides whether it lands, so tone is part of the work, not a nicety. Since the real payoff of review is shared understanding rather than pure defect-catching (Microsoft Research, 2013), a comment that makes the author defensive has already failed, no matter how correct it is.

Blockers and opinions get separated out loud. Small stuff gets a “nit:” prefix so the author knows it is optional, and a request for changes is reserved for things that are actually wrong. Asking beats demanding: “what happens here if the list is empty?” invites a fix, while “this is broken” invites an argument. And when something is good, say so. Approving a change without a word teaches nothing.

What I actually do: I aim to leave at least one comment that makes the author a better engineer, not just the code better. That is the difference between a gate and a mentor, and it is the half of reviewing that most guides skip.

If you want that kind of feedback on your own code, or you are trying to build a review culture that grows people instead of grinding them down, that is exactly what my code and architecture reviews and mentoring are for. The same instinct shows up in how I run one-to-one meetings as an engineering lead.

What I try not to comment on

Knowing what to ignore is a senior skill, and it is where most reviews go wrong. A review has only a short effective window before attention fades, so every comment spent on a variable name is a comment not spent on a real defect. Nitpicking is usually a symptom of reviewing in the wrong order: if I am arguing about formatting, I probably have not found the actual problem yet.

So I automate the nits out of existence. On Android that means ktlint or Spotless for formatting, detekt for code smells, and Android Lint in CI. If a machine can catch it, a human should never spend a review comment on it. That frees the review for the things only a person can judge: whether the design is right, whether the edge cases hold, whether the next engineer will understand it.

Whatever is left, I hold loosely. A naming preference is a “nit:”, not a blocker. “I would have done it differently” is not a reason to request changes unless different is genuinely better, and I have to be able to say why. Style is the linter’s job. Judgement is mine.

My Android code review checklist

The order I read a pull request in:

  • Correctness: nullability, threading, coroutine scope, lifecycle, the empty and error states.
  • Architecture: where the code lives, what it now depends on, and whether the next feature gets easier or harder.
  • Runtime: main-thread work, leaks, configuration changes, Compose recomposition and stability, list performance.
  • Tests: do they fail when behaviour breaks, and do they read like intent rather than mock choreography?
  • Reviewability: is the diff under about 400 lines, with a description that says what and why?
  • Style: naming and formatting last, and mostly left to the linter.

Frequently Asked Questions

What is the most important thing to check in an Android code review? Correctness comes first: null-safety, threading, coroutine scope and lifecycle behaviour. Kotlin helps here, with Google reporting 33 percent fewer NullPointerException crashes at Google Home after adopting it, but the compiler cannot verify that the change actually does what the ticket asked, so a human still has to.

How big should a pull request be for a good review? Keep a single review under roughly 400 lines of code. The Cisco study behind SmartBear’s guidance found defect-finding drops off sharply beyond that, and that reviews of 200 to 400 lines over 60 to 90 minutes find 70 to 90 percent of defects. If a change is bigger, ask for it to be split before reviewing.

How fast should code reviews be? Google’s engineering guidance sets one business day as the maximum to respond to a review request, and stresses that a fast response matters more than a fast completion. Elite teams, per LinearB’s benchmarks, keep pickup time under seven hours. A quick partial review that unblocks someone beats a thorough review that lands a day late.

Does code review replace testing on Android? No. Review and tests catch different classes of defect, and the best reviews check that tests exist and are meaningful rather than assuming coverage numbers tell the story. GitClear’s 2025 data shows duplication rising and refactoring falling, which is exactly the kind of maintainability erosion review catches and tests do not.

How do you give code review feedback without demoralising people? Separate blockers from opinions, label small suggestions as optional nits, ask questions instead of issuing verdicts, and acknowledge good work. Microsoft’s research found review delivers most of its value through knowledge transfer, so a comment that teaches something is worth more than one that only scores a point.

Is nitpicking bad in code reviews? Nitpicking is not evil, but it is misplaced when a machine could do it. Formatting and style belong to tools like ktlint, Spotless, detekt and Android Lint, not to human review comments. Label genuine suggestions as optional nits, and reserve requests for changes for real problems. If you are debating a variable name, you probably have not found the actual defect yet.

The short version

Read for correctness before you read for style. Keep pull requests small enough to actually understand, turn them around fast, and remember that the person on the other end learns from every comment you leave. The checklist matters, but the order and the tone matter more. Get those right and code review stops being a gate and starts being one of the best teaching tools your team has.

If you want a second set of senior eyes on your Android or Kotlin Multiplatform codebase, that is what my code and architecture reviews are for.

Sources

Share:

Comments

Loading comments…