¿Cómo realizas una revisión de código (Code Review)?

Planteamiento y contexto aplicable

Tienes 30 minutos para revisar un pull request que no conoces. Explica cómo reconstruyes su intención, ordenas la revisión, separas los comentarios bloqueantes de los no bloqueantes, redactas observaciones sobre las que el autor pueda actuar y eliges entre aprobar, comentar y solicitar cambios.

Esta es una pregunta general de entrevista de ingeniería de software para roles de backend, frontend, mobile, infraestructura y gestión de ingeniería. Puede plantearse como una pregunta de proceso verbal o como una revisión en vivo de un diff proporcionado. Ambas modalidades evalúan si puedes encontrar los problemas que más importan a los usuarios y al sistema bajo un límite de tiempo, en lugar de maximizar la cantidad de fallas de formato que reportas.

Asume que puedes ver la descripción del pull request, el requerimiento vinculado, los archivos modificados y los resultados de las pruebas, pero no conoces la base de código y no puedes hacer preguntas continuamente al autor. Si el entrevistador proporciona condiciones diferentes, recalibra el riesgo y el alcance antes de revisar.

Qué evalúa el entrevistador

La primera señal es si reconstruyes lo que se supone que debe hacer el cambio. Sin un requerimiento, un contrato de API o un límite de fallas, los comentarios se reducen a preferencias. Una respuesta sólida lee el contexto del pull request y el código circundante, identifica el cambio de comportamiento y solo entonces avanza línea por línea.

La segunda señal es la priorización. La corrección, la corrupción de datos, la seguridad, la autorización, la concurrencia y la compatibilidad generalmente merecen atención antes que el nombrado o la estructura visual. El entrevistador quiere ver si puedes expresar la consecuencia y dedicar tiempo a la ruta de mayor riesgo.

La tercera señal es la evidencia. “Esto podría ser un error” es solo una sospecha. El feedback de alta calidad proporciona la condición que lo detona, la consecuencia observable y una forma de verificarlo. Cuando falta contexto, formula una pregunta precisa en lugar de disfrazar una suposición como una conclusión bloqueante.

Finalmente, el entrevistador evalúa la decisión de revisión y la comunicación. Separa las correcciones obligatorias, las sugerencias opcionales, las preguntas aclaratorias y los detalles menores (nits). En el resumen, indica qué cubriste, qué queda sin verificar y por qué aprobaste o solicitaste cambios. Discute el código y su impacto, no la capacidad del autor.

Preguntas para aclarar antes de responder

• ¿Es una pregunta de proceso verbal o una revisión de diff en vivo? Para la primera, presenta un método reutilizable. Para la segunda, dedica unos segundos a exponer el método y luego aplícalo a líneas reales en lugar de recitar una lista de verificación.
• ¿Qué contexto está disponible? Un requerimiento, un contrato de API y el código circundante te permiten verificar el comportamiento. Con una función aislada, expón supuestos y convierte los contratos desconocidos en preguntas.
• ¿El cambio afecta a un dominio de alto riesgo? Pagos, autorización, privacidad, migraciones y APIs públicas elevan el nivel de exigencia de la evidencia y pueden requerir expertos en el dominio. Una herramienta interna de bajo riesgo puede favorecer una mejora incremental más rápida.
• ¿Qué entregables espera el entrevistador? Los comentarios en línea, un resumen, una decisión de aprobación y recomendaciones de pruebas requieren asignaciones de tiempo diferentes. Confirma el resultado esperado antes de consumir los 30 minutos.
• ¿Es un trabajo normal o una corrección de emergencia? Una emergencia puede justificar un parche más acotado y trabajo de seguimiento posterior. No justifica ignorar un riesgo conocido de seguridad o de corrupción de datos.
• ¿Estás calificado para cada dominio afectado? Si el cambio involucra criptografía, privacidad o una migración de base de datos fuera de tu experiencia, revisa lo que puedas y solicita un revisor calificado en lugar de aprobar por exceso de confianza.

Estructura de respuesta de 30 segundos

“Primero establezco el objetivo del pull request, el cambio de comportamiento y el impacto de posibles fallas; luego reviso en dos pasadas. La primera pasada mapea el límite del cambio, el flujo de datos y las rutas de alto riesgo, priorizando corrección, seguridad, datos, concurrencia y compatibilidad. La segunda pasada verifica casos extremos, manejo de errores, pruebas, observabilidad, rendimiento y mantenibilidad. Cada comentario indica severidad, detonante, consecuencia y resultado esperado. Un bloqueador reproducible implica solicitar cambios; las sugerencias y detalles menores pueden acompañar una aprobación. Finalizo indicando el alcance revisado, los riesgos no verificados y el motivo de mi decisión”.

Respuesta detallada paso a paso

Establece primero la línea base. Lee el título, la descripción, el requerimiento vinculado, los cambios en la API o el modelo de datos y las pruebas existentes. Reformula el contrato en una sola oración: “Este cambio otorga a este usuario un nuevo comportamiento bajo estas condiciones, preservando estas garantías existentes”. Si no puedes escribir esa oración, adquiere contexto antes de hacer comentarios a nivel de línea porque todavía no tienes un estándar de corrección.

Mapea después el límite del cambio. Sigue las entradas, los cambios de estado, los efectos secundarios externos y las rutas de retorno en lugar de limitarte solo a las líneas resaltadas. ¿Qué llamadas reciben un parámetro nuevo? ¿Están una escritura en base de datos y el envío de un mensaje dentro del mismo límite de falla? ¿Cambió una respuesta pública, el formato de un evento o un valor de configuración por defecto? El resultado de esta pasada es un modelo de dónde entra la información, qué límites de confianza cruza, qué estado cambia y cómo puede fallar.

Para un bloque de tiempo de muestra de 30 minutos, utiliza 3 minutos para la intención, 7 minutos para los límites y rutas de alto riesgo, 12 minutos para la inspección detallada, 5 minutos para pruebas y salvaguardas operativas, y 3 minutos para los comentarios y la decisión. Esta es una asignación práctica para ajustarse al tamaño del diff y al riesgo. Su propósito es evitar pasar los primeros 20 minutos discutiendo nombres.

Usa este orden de riesgo en la primera pasada:

1. Comportamiento y corrección: ¿Cumple la ruta principal con el contrato? ¿Qué sucede con entradas vacías, solicitudes duplicadas, fallas parciales y reintentos?
2. Seguridad y datos: ¿La autorización ocurre antes de cruzar el límite de confianza? ¿Se exponen datos sensibles? ¿Puede una falla causar pérdida, duplicación o un estado irreversible?
3. Concurrencia y compatibilidad: ¿Pueden solicitudes simultáneas romper un invariante? ¿Siguen funcionando los clientes antiguos, los datos previos y las versiones mixtas durante un despliegue progresivo (rolling deployment)?
4. Límite arquitectónico: ¿Está la responsabilidad en el componente correcto o el cambio elude una restricción existente y duplica estado?

La segunda pasada examina el detalle de implementación: flujo de control y propagación de errores, liberación de recursos, escala de consultas o bucles, logs y métricas, si las pruebas realmente fallarían cuando el código sea incorrecto, y si los nombres y comentarios ayudan a futuros lectores. Deja el formato y el estilo corregible automáticamente para el final, de modo que los detalles menores detectables por herramientas no desplacen al juicio humano.

Para cada hallazgo, verifica que el cambio lo haya introducido o expuesto. Si el nuevo código lee items[0] cuando items vacíos son válidos, se trata de una regresión concreta. Si el mismo archivo contiene complejidad preexistente no relacionada, menciónala como deuda técnica o crea una tarea de seguimiento a menos que su interacción con este cambio genere un riesgo de seguridad o de corrección. Una revisión no puede expandirse sin un límite.

Utiliza cuatro intenciones de comentario:

• Blocker (Bloqueador): La evidencia muestra una violación de contrato, un resultado incorrecto, un problema de seguridad, corrupción de datos o un riesgo de compatibilidad inaceptable. Debe resolverse antes de fusionar (merge).
• Question (Pregunta): Falta contexto que podría cambiar la conclusión. La respuesta puede cerrar la inquietud o promoverla a Blocker.
• Suggestion (Sugerencia): Una mejora de diseño, mantenibilidad u operativa que vale la pena, mientras que la implementación actual aún cumple con el estándar para mergear.
• Nit (Detalle menor): Un detalle no bloqueante de legibilidad o consistencia que el formateo o el análisis estático deberían gestionar idealmente.

Un comentario procesable contiene “etiqueta + condición + consecuencia + resultado esperado”, seguido de una posible dirección cuando sea útil. Por ejemplo:

Blocker: Cuando la solicitud permite items=[], leer items[0].id aquí lanza una excepción y el endpoint por lotes devuelve 500. Por favor maneja el arreglo vacío antes del bucle y agrega una prueba de regresión; si se debe devolver un resultado vacío o un 400 depende del contrato de la API.

Si no sabes si una entrada vacía es válida, plantéalo como una pregunta: “¿El contrato de la API permite un arreglo vacío? La ruta actual devuelve 500; si está permitido, necesita manejo explícito y una prueba”. Esto reporta la evidencia sin inventar un requerimiento.

Finaliza con una decisión de revisión. Selecciona solicitar cambios cuando haya un Blocker sin resolver. Envía un comentario cuando falte contexto esencial en lugar de ocultar la incertidumbre detrás de una aprobación. Aprueba cuando solo queden sugerencias no bloqueantes y aclara que no son condiciones para el merge. El resumen debe nombrar el alcance revisado, los hallazgos clave, la evidencia en tiempo de ejecución o de pruebas, los dominios no cubiertos y el estado final.

Un CI en verde no demuestra que la revisión esté completa. Las pruebas pueden omitir la rama crítica y las herramientas estáticas no conocen el contrato del producto. A la inversa, la revisión humana no debe reemplazar a las pruebas ejecutables. Conecta ambas: identifica una condición que falla en el comentario y solicita una verificación que falle antes del arreglo y pase después de él.

Ejemplo de respuesta de alta calidad

“No comenzaría buscando fallas a nivel de línea. Leería la descripción del pull request, el requerimiento vinculado y los cambios de interfaz; luego reformularía el objetivo y el comportamiento anterior que debe mantenerse. Si no hay contexto disponible, enumeraría los supuestos en lugar de presentar suposiciones como bloqueadores.

Dentro de los 30 minutos, utilizaría dos pasadas. La primera sigue los puntos de entrada, los cambios de estado, los efectos secundarios externos y las rutas de retorno, priorizando corrección, seguridad, corrupción de datos, concurrencia y compatibilidad. La segunda cubre casos extremos, manejo de errores, rendimiento, logs, pruebas y mantenibilidad. El estilo queda para el final, y solo cuando las herramientas automáticas no hayan cubierto un problema que realmente afecte la comprensión.

Cada hallazgo debe responder a tres preguntas: qué lo detona, cuál es la consecuencia y cómo verificarlo. Etiqueto las correcciones obligatorias como Blocker, el contexto faltante como Question, las mejoras no bloqueantes como Suggestion y los detalles estéticos como Nit. Por ejemplo, si una ruta con un arreglo vacío lee el primer elemento a pesar de que la API acepta entradas vacías, explicaría que devuelve 500 y solicitaría manejo explícito más una prueba de regresión, en lugar de escribir simplemente ‘posible problema de null’.

Antes de enviar, compruebo que cada comentario se relacione con este diff, que no haya elevado una preferencia personal a regla y que no falte ningún revisor de dominio requerido. Un problema reproducible de seguridad, datos o corrección conduce a solicitar cambios; las sugerencias por sí solas pueden acompañar una aprobación. Mi resumen enumera los archivos cubiertos, la evidencia de pruebas, las áreas no verificadas y el fundamento de la decisión para que el autor conozca la siguiente acción y los revisores posteriores sepan qué revisé en realidad”.

Errores comunes

• Abrir el diff y comentar línea por línea → Sin la intención o el contrato, las compensaciones válidas parecen errores → Reformula primero el objetivo, el cambio de comportamiento y el límite de fallas.
• Comentar en orden de descubrimiento → Los detalles de nombrado pueden sepultar riesgos de corrupción de datos o de autorización → Ejecuta la pasada de riesgos antes del pulido de implementación.
• Escribir solo “esto podría ser un bug” → El autor carece de un detonante y no puede verificar la corrección → Indica la condición, la consecuencia, la evidencia y el resultado esperado.
• Hacer que cada comentario sea obligatorio → El autor no puede distinguir el estándar para el merge de una preferencia → Etiqueta explícitamente Blocker, Question, Suggestion y Nit.
• Recitar una lista de verificación larga para parecer minucioso → Una lista no aplicada al flujo de datos demuestra poco criterio → Rastrea una ruta crítica y explica la cobertura restante.
• Exigir que se arreglen todos los problemas antiguos → El pull request se expande sin riesgo acotado ni validación → Separa las regresiones de la deuda existente, excepto donde se combinen en un problema de seguridad o corrección.
• Tratar el CI en verde como evidencia de aprobación → Las pruebas pueden codificar el contrato incorrecto u omitir una rama → Verifica si fallan ante el contraejemplo clave y solicita la prueba de regresión faltante.
• Comentar sobre el autor en lugar del código → Genera una actitud defensiva y no aporta justificación técnica → Describe el código, la condición y el impacto asumiendo buenas intenciones.
• Aprobar fuera de tu área de experiencia → La aprobación crea una falsa sensación de seguridad → Declara tu cobertura y solicita un revisor del dominio correspondiente.

Preguntas de seguimiento y respuestas

Pregunta de seguimiento 1: ¿Qué pasa si el autor disputa tu Blocker?

Vuelve al contrato verificable y a la consecuencia. Revisa si están en desacuerdo sobre las entradas, el riesgo o las condiciones de lanzamiento, y utiliza una reproducción mínima cuando sea posible. Si la evidencia no genera consenso, pide al dueño del código o al líder del dominio que decida y registra cualquier conclusión verbal en el pull request. No dejes el desacuerdo abierto indefinidamente.

Pregunta de seguimiento 2: ¿Qué pasa si el pull request es demasiado grande para terminarlo en 30 minutos?

No des a entender que hubo una cobertura total. Selecciona puntos de entrada, cambios de datos y contratos públicos según el riesgo; indica qué archivos revisaste línea por línea, cuáles escaneaste por encima o cuáles no cubriste; luego solicita dividir el PR o revisores de dominio adicionales. El estado de aprobación debe coincidir con la cobertura efectivamente realizada.

Pregunta de seguimiento 3: ¿Qué pasa si encuentras un problema preexistente grave fuera del diff?

Primero determina si este cambio lo detona o lo amplifica. Bloquea cuando la interacción cree un riesgo de seguridad, datos o corrección para el lanzamiento actual. Si es independiente, registra la evidencia, crea una tarea de seguimiento de alta prioridad y notifica al responsable en lugar de forzar una refactorización ilimitada dentro de este pull request.

Pregunta de seguimiento 4: ¿Puedes aprobar una corrección de emergencia sin pruebas completas?

Establece el costo inmediato de no corregir, si el parche es acotado, la ruta de reversión (rollback) o feature flag, y la validación dirigida más pequeña disponible. Un proceso de emergencia explícito puede aceptar cobertura de seguimiento posterior, pero un riesgo conocido de seguridad, corrupción de datos o daño irreversible aún requiere un estándar de aprobación más alto. El reloj no lo aprueba automáticamente.

Pregunta de seguimiento 5: ¿Cómo revisas un dominio que no conoces?

Continúa verificando el flujo de control general, el manejo de errores, las pruebas y los cambios de interfaz mientras señalas aquello que no estás calificado para juzgar. Solicita al responsable adecuado para criptografía, privacidad, migraciones o concurrencia compleja. Una revisión parcial es valiosa solo cuando no se presenta como una aprobación completa.

Pregunta de seguimiento 6: ¿Aún necesitas leer cada línea cuando la cobertura de pruebas es amplia?

Sí, pero con un enfoque diferente. Las pruebas proporcionan evidencia ejecutable para los casos ya codificados; la revisión aún cuestiona si el requerimiento es correcto, si faltan riesgos por contemplar, si el diseño añade complejidad innecesaria y si el registro de logs o la compatibilidad son adecuados. Convierte los contraejemplos importantes encontrados en la revisión en pruebas para que la corrección futura no dependa de la memoria del revisor.