4. Nombrar un problema en una revisión para que sea accionable

Descripción

Al terminar esta lección vas a tener una técnica concreta y repetible para convertir un hallazgo en un comentario de revisión sobre el que la otra persona pueda actuar sin volver a preguntarte nada. La técnica tiene cinco partes —nombre, ubicación, consecuencia, dirección y peso— y vas a verla aplicada sobre comentarios reales, en versiones antes y después. Vas a salir también con lo contrario: el criterio para saber cuándo no escribir un comentario, que es una parte del oficio que casi nadie enseña y que separa a un buen revisor de uno agotador.

Esta es la lección técnica del módulo, y es la que más veces vas a releer. Las lecciones 2 y 3 te dieron el diagnóstico; esta te da la forma de entregarlo. Y la forma no es un detalle de estilo: es lo que decide si el problema que detectaste se arregla o no. Un diagnóstico impecable, mal entregado, produce exactamente el mismo resultado que no haber mirado el código: nada cambia. Peor todavía, produce un resultado neto negativo, porque gastó tiempo de dos personas y dejó a una de ellas con la sensación de haber sido evaluada sin saber para qué.

La medida de éxito de esta lección es una sola frase, y quiero que la tengas presente en cada ejemplo: ¿podría el autor actuar sobre este comentario sin volver a preguntar? No "¿está bien escrito?", no "¿es amable?", no "¿es correcto?". Un comentario puede ser correcto, amable y bien escrito y aun así dejar al autor sin saber qué hacer el lunes por la mañana. Esa es la prueba que vamos a aplicar todo el tiempo, y es la misma con la que se va a evaluar el proyecto del módulo.

Conexión con el módulo: las lecciones 2 y 3 fueron sobre ver; esta es sobre decir. Toma las tres piezas que la lección 2 dejó separadas —síntoma, hipótesis y costo— y las convierte en un formato de cinco partes que se escribe en un minuto. La lección 5 va a agregar al vocabulario los anti-patrones, que se comunican con esta misma fórmula. La lección 6 va a profundizar en la cuarta parte —la dirección— mostrando el camino completo, en pasos pequeños, hacia un patrón y hacia afuera de uno. Y la lección 7 se ocupa de lo que esta deja fuera a propósito: el tono, la asimetría entre revisor y autor, y el daño de etiquetar por etiquetar.

Un reporte de falla que sirve y uno que no

Trabajas en una empresa que tiene una flota de camionetas. Un conductor deja un reporte al final del turno.

Versión uno: "la camioneta 14 anda mal".

El mecánico que lo lee al día siguiente no puede hacer nada con eso. Va a tener que buscar al conductor —que hoy libra—, o subirse a manejar la camioneta un rato a ver si le pasa algo, o revisar todo por si acaso. La información existía en la cabeza del conductor y no llegó.

Versión dos: "camioneta 14: en frenadas fuertes desde más de 60 km/h se va hacia la derecha y suena un chirrido metálico del lado del copiloto. Empezó el martes. Con el freno de mano no pasa. Le puse aire a las llantas y sigue igual".

El mecánico levanta la camioneta, mira el lado derecho y en veinte minutos sabe qué pasa. Fíjate en lo que hizo el segundo reporte: dijo qué (se va a la derecha, chirrido metálico), dónde (lado del copiloto, frenadas fuertes desde alta velocidad), desde cuándo, y —esto es lo bueno— qué ya descartó (las llantas). Ninguna de esas piezas es difícil de escribir; lo que hace la diferencia es que el conductor pensó en el mecánico mientras escribía.

Ahora, una tercera versión que también existe y que es la más engañosa: "la 14 tiene un problema en el sistema de frenos, hay que cambiar las balatas del lado derecho". Suena mucho más profesional que la primera. Y puede ser correcta. Pero el conductor no es mecánico: saltó del síntoma al diagnóstico y a la solución, y si se equivocó, mandó a alguien a cambiar unas balatas que estaban bien mientras el problema real —una manguera de freno colapsada— sigue ahí. Además, si el mecánico no está de acuerdo, ahora la conversación es sobre quién tiene razón en vez de sobre qué se observa.

Los tres reportes tienen su equivalente exacto en una revisión de código. "Esto está feo" es la camioneta 14. "Esto es un God object, hay que partirlo en tres clases" es el conductor recetando balatas. Y el reporte útil es el que dice qué se observa, dónde, qué consecuencia tiene, y ofrece una dirección sin cerrar la puerta a que el otro sepa algo que tú no.

La fórmula de cinco partes

Un comentario accionable tiene cinco partes. No siempre las cinco explícitas —a veces el peso se sobreentiende, a veces la ubicación es obvia porque el comentario está anclado a la línea—, pero si falta alguna, conviene notar cuál falta y por qué.

Parte	Qué aporta	Qué pasa si falta
Nombre	Comprime la estructura del problema en dos palabras y lo vuelve buscable y comparable	El comentario se alarga y el autor tiene que reconstruir la idea en su cabeza
Ubicación	Convierte una impresión en un hecho verificable	El autor no sabe dónde mirar y suele mirar el lugar equivocado
Consecuencia	Justifica que el problema importe; es lo que hace discutible el comentario	El comentario suena a preferencia personal, y se responde con otra preferencia
Dirección	Convierte el diagnóstico en trabajo posible	El comentario es un veredicto; el autor sabe que hay un problema y no qué hacer
Peso	Le dice al autor qué hacer hoy, en este PR	Todo comentario largo se lee como condición para aprobar, y el PR se atasca

Vamos parte por parte, porque cada una tiene su forma correcta y su forma inútil.

Uno: el nombre

El nombre es lo que compra brevedad. "Feature envy" comprime, en dos palabras, la observación de que un método usa datos ajenos, la hipótesis de que está en el lugar equivocado y la dirección general de mudarlo.

Tres reglas para usarlo bien.

El nombre nunca va solo. Un nombre suelto es una etiqueta, y una etiqueta es un veredicto. La lección 1 ya lo mostró: "esto es shotgun surgery" y nada más es un muro. El nombre entra acompañado de por lo menos la consecuencia.

Glosa el nombre en media línea. Nunca sabes quién va a leer el comentario. Alguien nuevo en el equipo, alguien que aprendió el oficio en otro idioma, alguien que sabe el concepto pero con otro nombre. Cuesta seis palabras evitar todo eso:

"…es feature envy: el método usa más datos de Order que propios."

Esa glosa no es condescendencia. Es lo que permite que el comentario funcione sin que nadie tenga que admitir que no conocía el término, cosa que en un equipo con jerarquías casi nadie hace.

Si no estás seguro del nombre, no lo uses; describe. Un nombre equivocado manda a alguien en dirección equivocada con toda confianza. Si dudas entre Factory y Builder, describe la estructura y deja que el nombre lo ponga quien lo sepa: "esto decide qué objeto construir según un campo; ¿tiene sentido moverlo a un solo lugar?". El módulo 1 ya avisaba: nombrar mal confunde más que no nombrar.

Dos: la ubicación

Este es el más fácil de cumplir y el que más se descuida.

Ubicación es archivo y línea, o archivo y nombre del símbolo. "En esta clase" no es ubicación cuando la clase tiene ciento cuarenta líneas. "En el manejo de errores" no es ubicación. En la mayoría de las herramientas de revisión el comentario ya va anclado a una línea, y eso resuelve el caso simple.

El caso que importa es el otro: cuando el problema está fuera del diff. La lección 2 lo mostró — el PR agregaba student a calculator.py y el problema estaba en refunds/policy.py, que el PR ni tocaba. Ahí la ubicación es obligatoria y además hay que decir que está fuera:

"Fuera del diff, pero consecuencia directa de este cambio: refunds/policy.py:15 y tickets/transfer.py:22."

Esa advertencia hace dos cosas. Le ahorra al autor treinta segundos de confusión buscando en su propio cambio algo que no está. Y, más importante, le comunica que sabes que le estás pidiendo algo fuera de su encargo, lo que cambia por completo cómo se recibe el pedido.

Cuando el problema es de dispersión —shotgun surgery— la ubicación es una lista, y esa lista es la evidencia:

"…toca cinco lugares: checkout.py:41, config.py:18, api/routes.py:66, el archivo nuevo del proveedor y reports/reconciliation.py:73."

Tres: la consecuencia

Esta es la parte que más comentarios omiten y la que decide si el comentario se atiende.

La consecuencia se dice en términos de trabajo futuro o de riesgo concreto, no de principios. Compara:

❌ Principio	✅ Consecuencia
"Esto viola el principio de responsabilidad única"	"Cuando cambie una tarifa y cuando cambie un texto de correo hay que tocar el mismo archivo; el mes pasado eso nos costó dos conflictos de merge"
"Esto está muy acoplado"	"Si mañana cambiamos el SDK de Stripe, hay que tocar checkout.py, que es donde no queremos equivocarnos"
"Hay duplicación"	"Son cuatro copias del mismo esqueleto; el que se olvide de una va a fallar solo con eventos grandes y solo en producción"
"Esto no escala"	"Con el quinto proveedor el if de checkout va a tener quince ramas y va a seguir estando en el camino de toda compra"

La columna izquierda no es incorrecta. Es indiscutible en el mal sentido: no se puede verificar ni refutar, así que la conversación se va a una discusión abstracta sobre si el principio aplica. La columna derecha se puede confirmar o desmentir en dos minutos, y si el autor tiene un dato que tú no —"eso ya no aplica, vamos a quitar Stripe el trimestre que viene"— la conversación avanza en vez de estancarse.

Tres formas de decir una consecuencia, de menos a más fuerte:

El escenario hipotético. "Cuando entre el quinto proveedor, va a haber que…". Sirve siempre y es la más común.

El modo de falla. "Si alguien olvida la lista de routes.py, la API rechaza un proveedor que el checkout sí sabe cobrar." Más fuerte, porque describe algo que se rompe.

El precedente. "Ya nos pasó con el sorted de PdfExporter, que quedó desalineado dos meses." Es la más fuerte de todas, porque no es una predicción sino un hecho histórico del equipo. Cuando tengas uno, úsalo: cambia por completo el peso de un comentario.

Y una nota sobre el silencioso, que es el que más hay que subrayar. Si el modo de falla no rompe nada —el reporte que muestra klarpay sin traducir— dilo explícitamente, porque la reacción natural del autor va a ser "pero funciona". Sí funciona. Ese es el problema.

Cuatro: la dirección

Una dirección es una opción concreta, ofrecida como opción. Ni un mandato ni un rediseño.

Las tres formas que funcionan:

La dirección con el nombre del patrón. "Mover la decisión a un registro en payments/: un diccionario nombre → constructor que los tres módulos consulten. Es lo que en el módulo 4 llamamos Factory." El patrón nombrado ahorra el párrafo de explicación.

La dirección mínima. No hace falta que la dirección sea el diseño final. Muchas veces la mejor es la más pequeña que quita el riesgo: "por lo pronto, con que la lista de proveedores viva en un solo archivo y los otros dos la importen, ya no se pueden desincronizar".

La dirección abierta. Cuando no tienes clara la mejor salida, decirlo es mejor que inventar una: "no tengo claro cuál es la forma limpia aquí; se me ocurren dos y las dos tienen problemas. ¿Lo pensamos juntos en diez minutos?". Esto no debilita el comentario: lo vuelve honesto, y es lo que la lección 7 va a desarrollar.

Y una que no funciona, aunque se ve todo el tiempo: el rediseño en el margen. Tres párrafos describiendo la arquitectura correcta, en un comentario de línea, sobre un PR de una funcionalidad. Aunque el rediseño sea bueno, el formato está mal: un rediseño se discute en un documento o en una llamada, no en el margen de un cambio ajeno. Si tu dirección no cabe en cuatro líneas, la dirección correcta es "esto amerita una conversación aparte, ¿abrimos un ticket?".

Cinco: el peso

El peso es la parte que más gente omite y la que más problemas evita. Es una etiqueta al principio o al final que dice qué se espera del autor en este PR: si tiene que cambiar algo antes de que el cambio entre, si es una propuesta que puede declinar, o si es algo menor que puede ignorar sin dar explicaciones.

La taxonomía de etiquetas no es de esta guía y no la vamos a reinventar aquí. La define clean-code-and-code-review-guide en su módulo 4, lección 5, con tres etiquetas —bloqueo, sugerencia y nit— y con la prueba concreta para decidir cuál corresponde a cada comentario. Si tu equipo todavía no etiqueta, ese es el lugar al que ir. En los ejemplos de este módulo vas a ver el peso escrito en prosa ("bloqueante", "nota, no bloqueante") porque así se escribe en un PR real, pero el criterio para asignarlo viene de allá.

Lo que sí es de aquí es qué le hace el peso a un olor nombrado, y son dos observaciones.

La primera: si no marcas el peso, el autor lo va a inferir del largo. Un comentario de seis líneas sin etiqueta se lee como bloqueante aunque no lo sea, porque nadie escribe seis líneas sobre algo que no importa. Por eso el olor estructural que tú marcarías como "nota" es justamente el que más necesita la etiqueta: es el más largo.

La segunda: el peso es lo que te permite señalar problemas grandes sin frenar el trabajo del equipo. Ese es el desbloqueo más importante de esta lección. Sin la etiqueta, la única forma de mencionar un God object es convertir el PR de otra persona en un rediseño. Con la etiqueta, puedes dejar escrito el diagnóstico completo, con toda su evidencia, y aprobar el PR el mismo día. El hallazgo queda registrado, el trabajo fluye, y la decisión de cuándo atacarlo la toma el equipo.

Hay una convención publicada que formaliza esto y que vale la pena conocer, Conventional Comments: prefijos como issue:, suggestion:, question:, nitpick:, praise:, con un (blocking) opcional. Es de donde sale la taxonomía de la otra guía, y ahí está explicado cómo adoptarla.

La plantilla

Junta las cinco y queda esto. No es una camisa de fuerza; es un recordatorio de qué revisar antes de mandar.

[peso] `archivo:línea` — Esto es <nombre del olor>: <glosa en media línea>.

<Consecuencia: qué va a doler, cuándo, o qué se rompe si se olvida algo.
 Si hay precedente en el equipo, va aquí.>

<Dirección: una opción concreta, en cuatro líneas o menos.
 Si excede el PR, decirlo y proponer ticket.>

Ejemplo trabajado: cinco comentarios, antes y después

Estos cinco son los tipos que más aparecen. En cada uno, la versión "antes" es real en el sentido de que es lo que se escribe todos los días, no una caricatura.

Caso 1 — el que no dice nada

Antes:

"Creo que esto se podría simplificar."

Después:

Sugerencia — checkout/checkout.py:22-35. Este bloque mezcla dos alturas distintas: el cálculo del subtotal (lógica de negocio) y la construcción del cliente HTTP del proveedor (detalle de infraestructura). Leerlo obliga a cambiar de contexto en medio de la función.

Con extraer las dos partes a _calculate_total(order, coupon) y _charge(order) la función principal queda como una lista de pasos legible de corrido. Son unas diez líneas movidas, sin cambio de comportamiento.

Qué cambió: la versión "antes" no tiene ninguna de las cinco partes. Ni siquiera dice qué es lo que se podría simplificar. La versión "después" tiene ubicación exacta, el problema descrito (niveles de abstracción mezclados), la consecuencia (leerlo obliga a cambiar de contexto), una dirección concreta con nombres de función propuestos, y el peso. Y fíjate en la última frase: "sin cambio de comportamiento". Decirlo baja el costo percibido de aceptar la sugerencia, porque el autor sabe que no tiene que volver a probar todo.

Caso 2 — el nombre suelto

Antes:

"God object."

Después:

Nota, no bloqueante — checkout/checkout.py (todo el archivo, no este cambio en particular). Este archivo es un God object: concentra cuatro responsabilidades sin relación entre sí —precio, asientos, cobro y avisos— y cada una cambia por pedido de un área distinta. git log dice que apareció en trece de los últimos doscientos commits, o sea que uno de cada quince cambios del sistema pasa por el archivo donde menos queremos equivocarnos.

Este PR no lo empeora de forma significativa; lo menciono porque es la tercera vez este mes que un cambio de otra área tiene que tocarlo. Cuando haya espacio, el primer corte que rinde es sacar el bloque de cobro a una Factory en payments/: son veinte líneas menos y le quita una de las cuatro razones de cambio. Abro un ticket y lo dejo ahí.

Qué cambió: la versión "antes" es correcta y es inútil. La de después convierte una etiqueta en un caso con evidencia (git log, "tercera vez este mes"), aclara que este PR no es el culpable —lo cual evita que el autor se sienta acusado por algo que heredó—, y ofrece el primer paso más pequeño en vez del rediseño completo. Y el peso, explícito desde la primera palabra, permite que todo eso se diga sin frenar nada.

Caso 3 — el rediseño en el margen

Antes:

"Yo aquí haría una jerarquía de clases TicketKind con un método abstracto price(), otro refund_window() y otro is_transferable(), y usaría una Factory para instanciar la correcta a partir del campo kind. También habría que mover COURTESY_LIMIT a la clase de cortesía, y probablemente convenga que Ticket reciba su TicketKind en el constructor en vez de guardar el string. Con eso, calculate_price desaparece y refunds/policy.py también."

Después, partido en dos comentarios:

Bloqueante — refunds/policy.py:15 y tickets/transfer.py:22 (fuera del diff, pero consecuencia directa). Estos dos archivos también deciden según ticket.kind. Con este PR, student cae en su else: catorce días de ventana de reembolso y transferencia libre, decidido por omisión y sin que falle nada. ¿Es lo que quiere producto? Si sí, agreguemos la rama explícita de todos modos para que la decisión quede escrita.

Nota, no bloqueante — el patrón de fondo. ticket.kind es un str sobre el que ramifican tres archivos que no se conocen entre sí (pricing, refunds, transfer). Es primitive obsession con switch repetido encima, y el modo de falla es el que acabamos de ver: un tipo nuevo se comporta como general sin que nadie lo decida.

Tengo una idea de hacia dónde iría —mover las tres preguntas a un tipo propio por kind— pero es un rediseño del modelo de boletos y no cabe en este PR ni debería. ¿Lo vemos en la próxima sesión de diseño? Abro el ticket con esta nota.

Qué cambió: el "antes" no está mal pensado. Está mal ubicado: es un rediseño de sistema escrito en el margen de un PR que agregaba un tipo de boleto. El autor lee eso y solo tiene dos salidas, las dos malas: pelearse o rediseñar el modelo de datos en un PR que no era eso. La versión de después separa lo urgente y pequeño —dos ramas explícitas, bloqueante— de lo estructural y grande —una nota con propuesta de conversación—. Es la misma información, entregada de forma que se puede actuar sobre ella.

Caso 4 — el principio en vez de la consecuencia

Antes:

"Este método viola la ley de Demeter."

Después:

Sugerencia — reports/reconciliation.py:88. order.customer.event.organizer.email es una message chain: esta línea depende de cuatro estructuras para conseguir un correo. Si cualquiera de los cuatro eslabones cambia —y Customer está en la lista de refactors del trimestre— esta línea se rompe, y se rompe en tiempo de ejecución dentro de un envío de correo, que es un lugar incómodo para explotar. Además, hoy revienta con AttributeError si la orden no tiene evento asociado, que pasa con las órdenes de prueba.

Lo mínimo sería order.organizer_email() en Order, que hace el recorrido en un solo lugar y puede devolver None en vez de explotar.

Qué cambió: el "antes" cita una regla. Quien la conoce piensa "ah, sí" y no hace nada; quien no la conoce se siente examinado. La versión de después no menciona la regla en absoluto: menciona el nombre del olor, describe la dependencia real (cuatro estructuras), da un modo de falla futuro (el refactor de Customer) y uno presente (AttributeError hoy), y propone lo mínimo. El principio sigue estando debajo de todo esto — simplemente no hacía falta invocarlo.

Caso 5 — la pregunta pasivo-agresiva

Antes:

"¿En serio vamos a tener otra lista de proveedores más?"

Después:

Bloqueante — api/routes.py:66. Esta es la segunda lista de proveedores válidos; la otra es el if/elif de checkout.py:41. Si las dos se desincronizan —y con este PR ya hay que acordarse de las dos— la API puede rechazar un proveedor que el checkout sabe cobrar, o al revés, y el error va a apuntar al lugar equivocado.

Para este PR alcanza con que ALLOWED_PROVIDERS se calcule desde un solo lugar. Si payments/ expone un available() que devuelva las claves, routes.py lo importa y esta lista desaparece. Son unas cinco líneas.

Qué cambió: el "antes" tiene un diagnóstico correcto escondido dentro de una expresión de frustración. Y la frustración es entendible —probablemente es la tercera vez que pasa—, pero convierte el comentario en algo a lo que el autor solo puede responder a la defensiva. La versión de después dice exactamente lo mismo sin el "en serio": nombra el problema, da el modo de falla, y ofrece un arreglo de cinco líneas que hace que el pedido sea razonable como bloqueante. La lección 7 vuelve sobre esto en serio.

Qué esperar de estos cinco casos. Lo primero, y es lo que quiero que midas: las versiones "después" no son más duras que las "antes". En dos de los cinco casos son claramente más suaves. Lo que son es más útiles, y esas dos cosas no están relacionadas. La dureza percibida de un comentario viene del tono y de la ausencia de contexto, no de la cantidad de información técnica.

Lo segundo: cuatro de los cinco casos "después" son más largos, y aun así se leen más rápido, porque el autor no tiene que descifrar nada. El único costo real está en escribirlos, y con la plantilla ese costo es de un minuto por comentario. En un PR de nueve comentarios, nueve minutos.

Lo tercero, y es lo más importante para tu práctica: fíjate en cuántas de las versiones "después" incluyen una estimación del tamaño del arreglo —"son unas cinco líneas", "son diez líneas movidas", "sin cambio de comportamiento"—. Esa frase hace más por que un comentario se atienda que cualquier argumento de diseño. La razón por la que la gente ignora comentarios de estructura casi nunca es que no esté de acuerdo: es que no sabe cuánto le va a costar y asume lo peor.

Cuándo no escribir el comentario

Un buen revisor no es el que encuentra más cosas: es el que hace que se arreglen más cosas. Y hay una relación inversa muy real entre el número de comentarios y la probabilidad de que cada uno se atienda. Veinte comentarios en un PR producen menos cambio que cinco, porque el autor entra en modo de despacho: resuelve lo mecánico y marca el resto como visto.

Cuatro casos donde lo correcto es callarse, o decirlo en otro lado.

Cuando no estás seguro y no lo puedes verificar en dos minutos. Si no hiciste el grep, si no leíste los otros usos, si no sabes si esa clase es un DTO — no escribas el diagnóstico. Escribe la pregunta: "¿Order tiene comportamiento propio o es solo un contenedor de datos? Si es lo segundo, ignora lo que sigue."

Cuando el arreglo es más caro que el olor. Un olor real, con costo real, puede seguir sin valer la pena. Si tu propia estimación honesta es que arreglarlo toma dos días y el dolor que quita son diez minutos al trimestre, el comentario correcto es ninguno —o una nota en un documento de deuda técnica, que no es lo mismo que un comentario en el PR de alguien—.

Cuando ya dijiste lo mismo en otro comentario del mismo PR. Si el mismo olor aparece en cuatro lugares, escribe un comentario con la lista de los cuatro. Cuatro comentarios idénticos se leen como insistencia.

Cuando el problema es del equipo y no de la persona. Este es el más importante. Si el PR sigue una convención que el equipo estableció y que a ti te parece mala, el lugar para discutirlo no es el margen del código de alguien que hizo lo que se acordó. Eso va a una conversación de equipo. Convertir un desacuerdo de convención en un comentario de revisión es de las formas más rápidas de quemar la relación con un compañero.

Y el caso simétrico, que también hay que decir: hay un comentario que casi nunca se escribe y debería escribirse más, que es el que señala algo que está bien. "Me gusta que hayas dejado el ChargeResult en vez de devolver el dict del SDK; eso mantiene la traducción en un solo lado." No es cortesía: es información. Le dice al autor qué decisión suya valoró el equipo, que es exactamente el tipo de dato que hace falta para calibrar las siguientes.

La frontera, otra vez

Esta lección se metió deliberadamente en terreno cercano a clean-code-and-code-review-guide, y conviene marcar dónde está la línea.

Lo que es de aquí: el contenido técnico del comentario. Qué nombre usar, qué evidencia citar, cómo expresar la consecuencia en términos verificables, qué dirección proponer y cómo dimensionarla. Todo eso depende del vocabulario de patrones y olores, y por eso vive en esta guía.

Lo que es de allá: el proceso alrededor. Cuántos comentarios es razonable dejar en un PR según su tamaño, cuándo aprobar con comentarios y cuándo bloquear, qué hacer cuando el autor no está de acuerdo, cuándo mover la discusión a una llamada, cómo se acuerda una convención de equipo, cuánto tiempo puede quedarse un PR abierto, y qué se automatiza con un linter para que nunca llegue a un comentario humano.

La única de esas que tocamos aquí es el peso, y solo porque sin él la información técnica no se puede entregar sin frenar el trabajo. Todo lo demás sobre el flujo del review hay que ir a buscarlo a la otra guía.

Errores comunes

Escribir la fórmula como formulario (de estilo). Qué pasa: alguien aprende las cinco partes y empieza a producir comentarios que dicen literalmente "Nombre: God object. Ubicación: checkout.py. Consecuencia: …". El contenido es correcto y el efecto es horrible: se lee como un informe de auditoría, y el autor —que es una persona— siente que le levantaron un acta. Por qué pasa: una plantilla invita a llenarla. Cómo detectarlo: si tu comentario tiene encabezados o viñetas para las cinco partes, ahí está. Cómo corregirlo: las cinco partes son un chequeo, no un formato. Se escriben en prosa corrida de tres o cuatro líneas, en el orden que fluya. Y varias veces una parte se puede omitir: si el comentario está anclado a la línea, la ubicación ya está; si el peso es obvio por el contexto, sobra la etiqueta. Relee y pregúntate qué falta, no qué casilla no llenaste.

Convertir la consecuencia en una profecía (de precisión). Qué pasa: alguien aprende que la consecuencia hace fuerte al comentario y empieza a inflarla: "esto nos va a costar semanas", "esto es una bomba de tiempo", "esto va a tirar producción". Casi nunca es cierto, el autor lo sabe, y el efecto es que aprende a descontar tus consecuencias — incluidas las verdaderas. Por qué pasa: la consecuencia es la parte persuasiva del comentario, y hay una tentación real de subir el volumen para que el problema se atienda. Cómo detectarlo: si tu consecuencia usa superlativos en vez de sustantivos concretos, la inflaste. "Va a tirar producción" no es una consecuencia; "si se olvida la lista de routes.py, la API rechaza órdenes válidas" sí lo es. Cómo corregirlo: exige que la consecuencia sea falsable. Escríbela de forma que el autor pueda responder "no, eso no pasa porque…". Si tu frase no admite esa respuesta, es retórica.

Omitir el peso y esperar que se entienda (de proceso). Qué pasa: alguien deja seis comentarios cuidados, sin etiquetas, tres de los cuales eran notas para el futuro. El autor, razonablemente, lee los seis como condiciones para aprobar, y responde con un PR de trescientas líneas donde había uno de cincuenta, o —más frecuente— se paraliza y el PR se queda tres días quieto. Por qué pasa: quien escribe tiene clarísimo en la cabeza cuáles eran importantes, y esa jerarquía no viaja con el texto. Además, poner "no bloqueante" se siente como debilitar el propio comentario. Cómo detectarlo: si un PR tuyo revisado se estancó y no sabes por qué, esta es la primera hipótesis a revisar. Cómo corregirlo: marca el peso siempre, aunque el equipo no tenga convención; con escribir "no bloqueante" o "bloqueante" al principio alcanza. Y recuerda el efecto que esto desbloquea: el peso es lo que te deja señalar un God object y aprobar el PR el mismo día. Sin él, tienes que elegir entre las dos cosas.

Ejercicios

Ejercicio 1 — Reescribe tres comentarios. Aquí hay tres comentarios reales sobre PRs de Boletia. Reescríbelos con las cinco partes. Inventa la evidencia concreta que haga falta —es parte del ejercicio darte cuenta de qué evidencia falta—.

(a) "Esta clase está haciendo demasiadas cosas." (b) "¿No convendría un patrón aquí?" (c) "Esto ya lo tenemos en otro lado, ¿no?"

Ver solución

Versiones posibles. Lo que importa es que estén las cinco partes y que la consecuencia sea falsable.

(a) El original tiene una hipótesis y nada más: ni ubicación, ni consecuencia, ni dirección, ni peso.

Sugerencia — notifications/manager.py, clase NotificationManager. La clase recibe seis dependencias en el constructor, pero ningún método usa más de tres, y los conjuntos no se solapan: send_order_confirmation usa db, mailer, sms_client y push_client; record_delivery usa solo db y analytics. Eso sugiere que aquí hay dos clases juntas —el envío y el registro de entregas— compartiendo constructor.

La consecuencia práctica hoy: cualquier prueba de una de las dos tiene que construir las seis dependencias. Partirla en dos clases deja pruebas de tres líneas. No urge; si no quieres tocarlo en este PR, con abrir un ticket estamos bien.

(b) El original es el peor de los tres, porque parece feedback y no lo es. Además delega en el autor tanto el diagnóstico como la solución.

Sugerencia — checkout/checkout.py:41-49. Este if/elif decide qué proveedor construir y ya tiene cuatro ramas. Cada rama sabe además cómo se construye el cliente del SDK y de dónde sale su credencial, así que el checkout depende de tres SDKs distintos para poder cobrar.

La consecuencia: el quinto proveedor vuelve a tocar este archivo, que es el que más miedo da del sistema, y una prueba del checkout tiene que poder construir los tres clientes aunque solo pruebe uno. Una dirección: un payments.provider_for(name) que encapsule la decisión y la construcción —lo que en el módulo 4 llamamos Factory—; el checkout queda con una línea y las pruebas pueden sustituir el proveedor. No bloqueante.

(c) El original tiene la intuición correcta y le falta lo más importante: el otro lado. "En otro lado" no es ubicación.

Bloqueante — reports/reconciliation.py:24-48. Esta secuencia (fetch → sort → format → write) es la misma que la de csv_exporter.py:8, pdf_exporter.py:8 y xlsx_exporter.py:8. Con este archivo son cuatro copias del mismo esqueleto, y solo cambia el paso del formato.

Lo marco bloqueante por un detalle concreto, no por la duplicación en sí: las otras tres pasan por sorted(rows, key=lambda r: r["name"]) y esta ordena por order_id, así que el reporte de conciliación va a listar los asistentes en un orden distinto al de los otros tres. Si es intencional, dejemos un comentario que lo diga; si no, alineémoslo. Aparte de eso, vale la pena subir el esqueleto a una base común (Template Method) — pero eso como nota, no para este PR.

Por qué funciona: los tres originales comparten el mismo defecto —tienen la parte que el revisor tenía en la cabeza y les falta todo lo que el autor necesita—. Fíjate en cuánta investigación hizo falta para escribir las versiones largas: contar dependencias por método, contar ramas, comparar cuatro archivos. Esa es la razón real por la que se escriben comentarios vagos, y por la que la lección 2 insistía tanto en el grep de dos minutos.

Ejercicio 2 — Decide el peso. Para cada uno de estos cinco hallazgos sobre el mismo PR, decide qué esperas del autor —¿bloquea la aprobación, es una propuesta que puede declinar, o es una nota que se deja escrita para después?— y justifica en una línea. El PR agrega el proveedor Klarpay a Boletia.

(a) El proveedor nuevo devuelve el dict crudo del SDK en vez de un ChargeResult como los otros tres. (b) checkout.py sumó un quinto elif. (c) El archivo nuevo usa comillas dobles y el resto del proyecto usa simples. (d) La prueba nueva hace Settings._instance = None para poder configurar el entorno. (e) No queda claro por qué el timeout de este proveedor es de 30 segundos y el de los otros de 10.

Ver solución

(a) Bloqueante. Es lo único de la lista que rompe algo hoy. Si charge() devuelve un dict donde el resto del sistema espera un ChargeResult, el if not result.ok del checkout va a fallar o —peor— evaluar la verdad de un diccionario no vacío y dar por buena una compra que falló. Es un bug, no un problema de estructura.

(b) Nota, no bloqueante. Es shotgun surgery real y bien diagnosticado, y no es culpa de este PR: el autor hizo lo único que se podía hacer con la estructura que hay. Bloquear aquí sería pedirle que rediseñe el manejo de proveedores para poder entregar una funcionalidad. Se documenta con evidencia, se abre ticket, se aprueba.

(c) Detalle, o ninguno. Y en realidad la respuesta correcta es que este comentario no debería existir: si al equipo le importa el estilo de comillas, eso lo pone un formateador automático en el pipeline. Cada comentario humano sobre algo que una herramienta puede arreglar es tiempo de dos personas quemado. Esa discusión —qué se automatiza y qué se comenta— es de clean-code-and-code-review-guide.

(d) Pregunta, que probablemente se vuelva nota. El Settings._instance = None no es un error del autor: es la única forma de probar contra un Singleton, y es el mejor argumento posible contra el Singleton de config.py. La forma útil de comentarlo es como pregunta: "¿esto es porque Settings no deja instanciar una configuración de prueba? Si es así, es el argumento que nos faltaba para pasarla a inyección; abro ticket." Bloquear el PR por esto sería castigar a alguien por un problema que heredó.

(e) Pregunta, genuina. No sabes si es un descuido o si el SDK de Klarpay de verdad es lento. Si es lo segundo, la respuesta del autor es información valiosa que conviene que quede escrita en un comentario del código. Si es un descuido, la pregunta lo revela sin acusar a nadie.

Lo que hay que ver en el conjunto. De cinco hallazgos, uno solo bloquea. Los otros cuatro se dicen igual —ninguno se calla— pero ninguno frena el PR. Ese reparto es lo normal en una revisión sana, y es exactamente lo que el peso permite. Un revisor sin la noción de peso tiene dos modos: aprobar callándose cuatro cosas, o bloquear por las cinco. Los dos son peores.

Por qué funciona: asignar peso te obliga a separar dos preguntas que se confunden todo el tiempo — "¿esto está mal?" y "¿esto tiene que cambiar antes de mergear?". Son independientes, y confundirlas es la causa más común de PRs atascados.

Ejercicio 3 — Escribe el comentario que no vas a escribir. Vuelve al hallazgo del ejercicio 1 de la lección 1 —esa cosa que viste y no dijiste—. Escríbela ahora con las cinco partes. Después contesta dos preguntas: (a) ¿cuánto tardaste?, y (b) ¿la razón por la que no lo dijiste sigue en pie con este formato?

Ver solución

No hay respuesta única, pero hay dos resultados que se repiten y vale la pena anticipar.

Sobre (a): la mayoría tarda entre tres y ocho minutos la primera vez, y casi todo ese tiempo se va en la consecuencia —porque exige pensar en un escenario concreto de cambio— y en la evidencia —porque exige ir a contar algo—. Esa es la buena noticia: la parte cara del comentario es la que solo se paga una vez, cuando investigas. Con el hallazgo ya investigado, escribirlo son treinta segundos.

Sobre (b), las cuatro razones típicas de la lección 1 y qué pasa con cada una:

• "No sabía cómo decirlo sin sonar agresivo." Normalmente se disuelve, y por una razón que sorprende: al obligarte a describir la consecuencia en términos de trabajo futuro, el formato saca al autor del centro de la frase. El comentario deja de hablar de lo que él hizo y pasa a hablar de lo que va a pasar. Eso baja la temperatura sin que tengas que suavizar nada.
• "Me iba a tomar mucho tiempo." Se disuelve parcialmente. Sigue costando lo que cuesta investigar, pero ya no cuesta redactar.
• "No estaba seguro de tener razón." No se disuelve con este formato, y no debería: si no estás seguro, la salida correcta no es escribirlo mejor, es escribirlo como pregunta. Eso es la lección 7.
• "No quería frenar el PR." Se disuelve completamente, y es el desbloqueo más grande de esta lección. Con el peso marcado, señalar un problema estructural y aprobar el PR el mismo día son compatibles.

Si al terminar el ejercicio tu comentario te sigue pareciendo imposible de mandar, hay dos posibilidades y conviene distinguirlas. Una: el problema es de equipo o de convención, y el margen de un PR es el lugar equivocado —lo vimos en "cuándo no escribir el comentario"—. Dos: hay algo en la dinámica del equipo que hace costoso disentir, y eso no lo arregla ningún formato; eso es material de la guía de code review y, honestamente, de una conversación con quien lidere el equipo.

Por qué funciona: el módulo entero apuesta a que el vocabulario baja el costo de decir la verdad completa. Este ejercicio es la medición de esa apuesta sobre un caso tuyo, no sobre un ejemplo inventado.

Resumen y siguiente paso

En esta lección convertiste el diagnóstico en entrega. Un comentario accionable tiene cinco partes: el nombre (que compra brevedad, nunca va solo, se glosa en media línea y no se usa si no estás seguro), la ubicación (archivo y línea, o la lista completa cuando el problema es de dispersión, y con la advertencia explícita cuando está fuera del diff), la consecuencia (en términos de trabajo futuro o de riesgo concreto, nunca de principios; y en su forma más fuerte, un precedente real del equipo), la dirección (una opción concreta, en cuatro líneas o menos, con estimación de tamaño) y el peso (que es lo que permite señalar un problema grande sin frenar el trabajo).

Viste cinco pares antes/después —el comentario vacío, el nombre suelto, el rediseño en el margen, el principio en vez de la consecuencia y la pregunta pasivo-agresiva— y comprobaste tres cosas: que las versiones útiles no son más duras, que se leen más rápido aunque sean más largas, y que la frase que más ayuda a que un comentario se atienda es la estimación del tamaño del arreglo.

Y viste el otro lado del oficio: los cuatro casos en los que lo correcto es callarse —cuando no puedes verificar, cuando el arreglo cuesta más que el olor, cuando repites el mismo hallazgo, y cuando el problema es del equipo y no de la persona— más el comentario que casi nadie escribe y debería escribirse más, que es el que señala lo que está bien.

Antes de avanzar deberías poder: enumerar las cinco partes y decir qué pasa si falta cada una; distinguir un principio de una consecuencia; y explicar por qué el peso es lo que permite diagnosticar un God object sin frenar el PR.

Lo que sigue amplía el vocabulario hacia un territorio distinto. Hasta aquí trabajamos con olores: señales locales en el código, que se detectan leyendo y contando. La lección 5 sube un nivel a los anti-patrones: soluciones que tienen nombre, que parecen buenas, que se eligieron a propósito —casi siempre con buenas intenciones— y que producen más problema del que resolvían. Singleton como estado global, la fábrica de fábricas, el objeto ancla que todos importan, la herencia de cinco niveles. Son más difíciles de señalar que un olor, precisamente porque alguien los defendió alguna vez con buenos argumentos.

Recursos

• Conventional Comments — la convención minimalista para marcar el peso y la intención de cada comentario. Se adopta en una reunión de quince minutos y resuelve la quinta parte de la fórmula.
• Google Engineering Practices — How to write code review comments — la guía de Google sobre el contenido y el tono de los comentarios. Complementa esta lección por el lado del proceso, que aquí no cubrimos.
• Refactoring Guru — Code Smells — útil para la parte "dirección": para cada olor lista las refactorizaciones asociadas, que es de donde salen las opciones concretas que propones.
• The art of giving and receiving code reviews gracefully (Alexandra Hill, LeadDev) — un ensayo corto y muy citado sobre la dimensión humana del review. Es la antesala de la lección 7 de este módulo.