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.
What a review returns
The 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).
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.
From experience: the 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 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.
I read the change against the ticket and ask blunt questions. Does it handle the empty state and the error state? 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. 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.
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?
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 reviewable?
A change I cannot hold in my head is a change I cannot review well, so size and description are part of 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).
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).
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. 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.
If you want that kind of feedback on your own code, or you are trying to build a review culture that grows people, 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 part of the review. 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.
Sources
- SmartBear, Best Practices for Peer Code Review, retrieved 2026-07-13, https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/
- Google, Android Developers Blog, “Google Home reduces its #1 cause of crashes by 33%”, retrieved 2026-07-13, https://android-developers.googleblog.com/2020/07/Google-home-reduces-crashes.html
- Google, “Build better apps with Kotlin” (crash-rate data across top 1,000 apps), retrieved 2026-07-13, https://developer.android.com/kotlin/build-better-apps
- GitClear, “AI Copilot Code Quality: 2025 Data Suggests 4x Growth in Code Clones”, retrieved 2026-07-13, https://www.gitclear.com/ai_assistant_code_quality_2025_research
- Google Research (Sadowski et al.), “Modern Code Review: A Case Study at Google”, ICSE-SEIP, retrieved 2026-07-13, https://research.google/pubs/modern-code-review-a-case-study-at-google/
- Google, “eng-practices: Speed of Code Reviews”, retrieved 2026-07-13, https://google.github.io/eng-practices/review/reviewer/speed.html
- LinearB, “Software Engineering Benchmarks: What Makes Elite Teams?”, retrieved 2026-07-13, https://linearb.io/blog/engineering-metrics-benchmarks-what-makes-elite-teams
- Microsoft Research (Bacchelli & Bird), “Expectations, Outcomes, and Challenges of Modern Code Review”, ICSE, retrieved 2026-07-13, https://www.microsoft.com/en-us/research/publication/expectations-outcomes-and-challenges-of-modern-code-review/
Quien intenta leerse un pull request de mil líneas de una sentada se va a dejar cosas. En el estudio de Cisco que hay detrás de la guía de revisión entre pares de SmartBear, la detección de defectos caía en picado en cuanto una sola revisión pasaba de unas 400 líneas de código (SmartBear, Best Practices for Peer Code Review). Ese único dato marca cómo me enfrento a una revisión de código Android.
A lo largo de doce años, en Sky, BritBox, BetMGM y con mis propios clientes, los vistos buenos rápidos nunca cazaron los problemas de verdad. Así que esta es la lista que recorro de cabeza cuando abro un pull request de Android. Empieza por la corrección y termina por el tono, y el orden importa más que la lista.
Ideas clave
- Las revisiones de menos de unas 400 líneas, en 60 a 90 minutos, encuentran entre el 70 y el 90 por ciento de los defectos (SmartBear).
- Reviso por orden de prioridad: corrección, arquitectura, comportamiento en ejecución, tests y, al final, el estilo. El estilo va el último a propósito.
- Después de adoptar Kotlin, Google Home recortó un 33 por ciento su principal causa de caídas, las NullPointerException (Google).
- Un pull request pequeño y bien descrito que vuelve en un día gana a una revisión perfecta que llega tres días tarde.
Qué devuelve una revisión
El retorno es la comprensión compartida. Cuando quienes investigan en Microsoft estudiaron la revisión de código moderna, vieron que encontrar defectos era la motivación declarada, pero en la práctica las revisiones aportaban más valor por la transferencia de conocimiento, la conciencia del equipo y mejores soluciones que por la detección de defectos en sí (Microsoft Research, Expectations, Outcomes, and Challenges of Modern Code Review, 2013).
Estoy ahí para asegurarme de que el cambio es correcto, de que una segunda persona lo entiende y de dejar a quien lo escribió un poco mejor de como lo encontré.
Desde la experiencia: el hábito que tengo es revisar por orden de prioridad y decirlo. Primero la corrección y la arquitectura, y al final los nombres y el formato. Cuando abres con una pega sobre el nombre de una variable, quien escribe corrige el nombre y deja de leer, y el bug de verdad, tres ficheros más abajo, no se discute nunca.
Si quieres esa misma disciplina de revisión aplicada a tu propio repositorio, es una de las cosas que hago en las revisiones de código y de arquitectura de uno a uno.
¿El código hace lo que dice el pull request?
La corrección va primero y, en Android, eso empieza por la seguridad ante nulos, los hilos y el ciclo de vida. El sistema de tipos de Kotlin se paga aquí de una forma medible. En 2020, el equipo de Google Home informó de un 33 por ciento menos de caídas por NullPointerException después de pasarse a Kotlin, y las NPE habían sido su causa número uno de caídas (Google, Google Home reduces its #1 crash cause by 33%, 2020). Google también ha dicho que las aplicaciones del top 1.000 de Google Play que usan Kotlin ven un 20 por ciento menos de caídas por usuario.
Leo el cambio contra el ticket y hago preguntas directas. ¿Gestiona el estado vacío y el estado de error? ¿Hay trabajo bloqueante en el hilo principal? ¿Las corrutinas están acotadas a algo que se cancela, o se escapan más allá de la pantalla que las lanzó? Kotlin hace que mucho de esto se lea bien, pero la nulabilidad, la cancelación y los tipos de plataforma siguen escondiendo bugs que solo ve una persona.
Para el lado de las corrutinas, he escrito cómo Job y SupervisorJob cambian el comportamiento de la cancelación, y para las características del lenguaje en las que más me apoyo, qué hacen de verdad inline, crossinline, noinline y reified. La charla de Romain Guy en la droidcon London 2024, Optimizing Kotlin Code in Practice, también es un gran recorrido por el Kotlin idiomático frente al Kotlin caro, que es justo la lente con la que miro esta parte de una revisión.
¿La arquitectura va en la dirección correcta?
La revisión de arquitectura va de dirección. El estudio de Google sobre su propio proceso de revisión, a lo largo de nueve millones de cambios revisados, vio que una cultura de revisión sana favorece cambios pequeños y de un solo revisor, en lugar de grandes debates de diseño pegados a un diff (Google Research, Modern Code Review: A Case Study at Google, 2018). Para cuando el código está en un pull request, la arquitectura está casi decidida, así que mi trabajo es cazar el cambio que, en silencio, apunta el código en la dirección equivocada.
Miro dónde vive el código y de qué depende ahora. ¿Esto pertenece al ViewModel, o la lógica de negocio se ha colado en el Composable o en la Activity? ¿Una clase de la capa de datos acaba de empezar a importar algo de la capa de UI? ¿El cambio añade un patrón nuevo cuando ya existen tres parecidos? También vigilo el problema contrario, que es la sobreingeniería: una abstracción genérica con un único llamante es un lastre.
Lo que hago de verdad: pregunto «¿la siguiente funcionalidad será más fácil o más difícil por este cambio?». Si la respuesta es más difícil, eso es un comentario de arquitectura, y pesa más que cualquier pega de estilo del diff.
Cuando un equipo decide qué compartir y qué dejar nativo, ese mismo razonamiento escala. Lo desarrollo en mi artículo de arquitectura limpia para Kotlin Multiplatform.
¿Cómo se va a comportar esto en un dispositivo real?
El código Android tiene que sobrevivir a un entorno de ejecución hostil, así que leo cada diff como si fuera a ejecutarse en un móvil de gama media, con una conexión inestable y un optimizador de batería agresivo. ¿El cambio hace trabajo de disco o de red en el hilo principal? ¿Guarda una referencia a un Context, una View o una Activity en un sitio donde puede sobrevivir a la pantalla y tener una fuga? ¿Qué pasa en un cambio de configuración, en la muerte del proceso o en una rotación rápida mientras la llamada de red sigue en vuelo?
Jetpack Compose añade sus propias preguntas de ejecución. ¿El estado está bien elevado, o un Composable lee y escribe el mismo estado de una forma que dispara recomposiciones de más? ¿Los tipos que se pasan a los Composables son estables, para que Compose pueda saltarse el trabajo que no necesita rehacer? ¿Una lista larga va respaldada por keys y un layout perezoso, o va a reconstruir el mundo entero en cada cambio? Nada de esto se ve en una captura, y por eso pertenece a la revisión y no a QA. Para las decisiones concretas de Compose a las que sigo volviendo, mira ConstraintLayout en Jetpack Compose.
¿Tiene tests, y envejecerá bien?
Compruebo que el cambio tiene tests y que el código no se va a pudrir, porque la revisión es uno de los pocos momentos en los que defender la mantenibilidad sale barato. El análisis de GitClear de 2025, sobre 211 millones de líneas cambiadas, vio que la refactorización bajó de cerca del 25 por ciento de las líneas cambiadas en 2021 a menos del 10 por ciento en 2024, mientras que las líneas copiadas y pegadas subieron del 8,3 al 12,3 por ciento (GitClear, AI Copilot Code Quality: 2025 Data, 2025). La duplicación se está colando otra vez en los códigos, y la revisión es donde la cazas.
En los tests no persigo un número de cobertura. Quiero que el test falle si el comportamiento se rompe, que se lea como documentación de la intención y que evite afirmar detalles de implementación que van a cambiar la semana que viene. Un test que mockea todo y afirma que se llamó a los mocks no demuestra nada. Prefiero tres tests que fijen los casos límite de verdad que treinta que inflen un informe de cobertura.
En la duplicación, marco la segunda copia, no la tercera. La primera vez que se copia código es perdonable, la segunda ya es un patrón, y a la tercera el coste de juntarlo ya se ha pagado varias veces.
¿Este pull request se puede revisar?
Un cambio que no me cabe en la cabeza es un cambio que no puedo revisar bien, así que el tamaño y la descripción son parte de la corrección. El estudio de Cisco que hay detrás de la guía de SmartBear es directo con los límites: mantén una sola revisión por debajo de unas 400 líneas, no inspecciones a más de unas 500 líneas por hora y no revises más de 60 minutos seguidos. Si te quedas dentro, una revisión encuentra entre el 70 y el 90 por ciento de los defectos presentes (SmartBear, Best Practices for Peer Code Review).
Si el diff es más grande que eso, lo digo antes de revisar y pido que se parta. También leo primero la descripción: qué ha cambiado, por qué y cómo se ha probado. Un pull request sin descripción es una petición de hacer ingeniería inversa de la intención de otra persona, y ahí es donde las revisiones se quedan en silencio y los defectos se cuelan. Para el lado de quien escribe, Best Practices for Pull Requests es un vídeo práctico y sólido.
¿Cómo de rápido debería volver una revisión?
Ir rápido es una funcionalidad, y una revisión lenta es un impuesto que paga todo el equipo. La propia guía de ingeniería de Google pone el techo en un día laborable para responder a una petición de revisión, y señala que responder rápido importa más que terminar toda la revisión de una vez (Google, Speed of Code Reviews). Los equipos de élite lo mantienen apretado: los benchmarks de LinearB, sobre más de ocho millones de pull requests, sitúan el tiempo de recogida de primer nivel por debajo de siete horas, el tiempo de revisión por debajo de seis y el tiempo de ciclo completo por debajo de 25 horas (LinearB, Engineering Benchmarks, de 2024 a 2026).
Por eso doy la vuelta a las revisiones rápido aunque no pueda hacer una pasada completa. Un comentario de dos líneas que desbloquea a alguien en una hora vale más que una revisión a fondo que aterriza mañana por la tarde.
Cómo doy la respuesta
Cómo digo algo decide si cala, así que el tono es parte del trabajo. Como el rendimiento real de la revisión es la comprensión compartida y no la caza pura de defectos (Microsoft Research, 2013), un comentario que pone a la defensiva a quien escribe ya ha fallado, por muy correcto que sea.
Los bloqueos y las opiniones se separan en voz alta. Lo pequeño lleva el prefijo «nit:» para que se sepa que es opcional, y una petición de cambios se reserva para lo que está mal de verdad. Preguntar gana a exigir: «¿qué pasa aquí si la lista está vacía?» invita a un arreglo, mientras que «esto está roto» invita a una discusión. Y cuando algo está bien, dilo. Aprobar un cambio sin una palabra no enseña nada.
Lo que hago de verdad: intento dejar al menos un comentario que convierta a quien escribe en mejor ingeniero.
Si quieres ese tipo de respuesta sobre tu propio código, o estás intentando construir una cultura de revisión que haga crecer a la gente, eso es justo para lo que sirven mis revisiones de código y de arquitectura, y la mentoría. El mismo instinto aparece en cómo llevo las reuniones uno a uno como responsable de ingeniería.
En qué intento no comentar
Saber qué ignorar es parte de la revisión. Una revisión solo tiene una ventana útil corta antes de que la atención se apague, así que cada comentario gastado en el nombre de una variable es un comentario que no se gasta en un defecto real. Ponerse puntilloso suele ser síntoma de revisar en el orden equivocado: si estoy discutiendo el formato, probablemente todavía no he encontrado el problema de verdad.
Así que automatizo las pegas menores hasta que desaparecen. En Android eso significa ktlint o Spotless para el formato, detekt para los malos olores del código y Android Lint en la CI. Si lo puede cazar una máquina, una persona no debería gastar nunca un comentario de revisión en ello. Eso libera la revisión para lo que solo puede juzgar una persona: si el diseño es el correcto, si los casos límite aguantan y si la siguiente persona que llegue al código lo va a entender.
Lo que queda lo llevo con calma. Una preferencia de nombres es un «nit:», no un bloqueo. «Yo lo habría hecho de otra forma» no es motivo para pedir cambios, salvo que lo distinto sea de verdad mejor, y tengo que ser capaz de decir por qué. El estilo es trabajo del linter. El criterio es mío.
Mi lista de revisión de código Android
El orden en el que leo un pull request:
- Corrección: nulabilidad, hilos, ámbito de la corrutina, ciclo de vida, y los estados vacío y de error.
- Arquitectura: dónde vive el código, de qué depende ahora y si la siguiente funcionalidad se vuelve más fácil o más difícil.
- Ejecución: trabajo en el hilo principal, fugas, cambios de configuración, recomposición y estabilidad de Compose, y rendimiento de listas.
- Tests: ¿fallan cuando se rompe el comportamiento, y se leen como intención en lugar de como coreografía de mocks?
- Revisabilidad: ¿el diff está por debajo de unas 400 líneas, con una descripción que dice qué y por qué?
- Estilo: nombres y formato al final, y casi todo dejado al linter.
Fuentes
- SmartBear, Best Practices for Peer Code Review, consultado el 2026-07-13, https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/
- Google, Android Developers Blog, “Google Home reduces its #1 cause of crashes by 33%”, consultado el 2026-07-13, https://android-developers.googleblog.com/2020/07/Google-home-reduces-crashes.html
- Google, “Build better apps with Kotlin” (datos de tasa de caídas en las 1.000 aplicaciones principales), consultado el 2026-07-13, https://developer.android.com/kotlin/build-better-apps
- GitClear, “AI Copilot Code Quality: 2025 Data Suggests 4x Growth in Code Clones”, consultado el 2026-07-13, https://www.gitclear.com/ai_assistant_code_quality_2025_research
- Google Research (Sadowski et al.), “Modern Code Review: A Case Study at Google”, ICSE-SEIP, consultado el 2026-07-13, https://research.google/pubs/modern-code-review-a-case-study-at-google/
- Google, “eng-practices: Speed of Code Reviews”, consultado el 2026-07-13, https://google.github.io/eng-practices/review/reviewer/speed.html
- LinearB, “Software Engineering Benchmarks: What Makes Elite Teams?”, consultado el 2026-07-13, https://linearb.io/blog/engineering-metrics-benchmarks-what-makes-elite-teams
- Microsoft Research (Bacchelli & Bird), “Expectations, Outcomes, and Challenges of Modern Code Review”, ICSE, consultado el 2026-07-13, https://www.microsoft.com/en-us/research/publication/expectations-outcomes-and-challenges-of-modern-code-review/
Loading comments…Cargando comentarios…