Comentarios de code review: tono, cortesía y precisión
En inglés escrito, la diferencia entre «change this» y «could we change this?» no es cortesía decorativa: es la que decide si el otro discute el código o se defiende a sí mismo. Las fórmulas son pocas y se aprenden de memoria.
Un comentario de revisión escrito por alguien que habla español suena, casi siempre, más duro de lo que esa persona quiso. No es maleducación: es que traducimos literal. «Esto está mal» sale como “this is wrong”, y en un equipo angloparlante eso se lee como un reto, no como una observación técnica.
Lo que falta no es vocabulario. Son tres o cuatro fórmulas fijas que suavizan la forma sin perder nada de precisión.
Por qué suena duro
El inglés profesional escrito usa mucho más el modo indirecto que el español rioplatense. Donde nosotros decimos «cambiá esto», el inglés dice “could we change this?” y se entiende exactamente igual de obligatorio. La cortesía no baja la exigencia: baja la temperatura.
| Traducción literal | Cómo se lee | Lo que se escribe |
|---|---|---|
| This is wrong | Un juicio sobre la persona | I think this breaks when the list is empty — worth a guard? |
| You forgot the null check | Una acusación | Looks like this can be null here — should we handle it? |
| Why did you do this? | Un reproche, aunque no lo sea | What's the reasoning behind this approach? I might be missing context |
| This is not readable | Un veredicto | I had to read this twice — would splitting it into two functions help? |
| Never use any here | Una orden seca | We avoid `any` in this repo — `unknown` plus a type guard would keep the same behavior |
| Fix this | Sin margen para responder | Could you fix this before we merge? |
Antes de seguir, predecí
Marcar qué bloquea y qué no
Un comentario ambiguo genera una ronda extra de revisión: el autor no sabe si tiene que cambiar algo o si puede mergear. Los equipos angloparlantes resuelven eso con prefijos, y son los mismos en todos lados.
| Prefijo | Qué significa | Ejemplo |
|---|---|---|
| blocking: | No se mergea hasta resolverlo | blocking: this drops the tenant filter, so accounts can read each other |
| nit: | Detalle menor, opcional | nit: typo in the comment above |
| question: | Pregunta genuina, no una crítica encubierta | question: is the retry safe if the job is not idempotent? |
| suggestion: | Una idea que el autor puede tomar o dejar | suggestion: `Array.from` would read a bit cleaner here |
| praise: | Algo que salió bien; se dice en voz alta | praise: the test names here are great |
| FYI / for context: | Información, sin pedido | FYI: we hit the same issue in #412 |
| Situación | Fórmula |
|---|---|
| Aprobar con detalles menores | LGTM with a couple of nits — feel free to merge after addressing them |
| Aprobar sin reservas | LGTM, nice work on the tests |
| Pedir cambios sin sonar tajante | A few things to sort out before this goes in — nothing structural |
| No entender el cambio | I'm not following this part — could you walk me through it? |
| Estar en desacuerdo | I'd lean the other way here, and here's why: … Happy to be convinced |
| Frenar un cambio riesgoso | blocking: this changes behavior for existing customers — can we put it behind a flag? |
| Responder a un comentario tuyo ya resuelto | Good catch, fixed in a1b2c3d |
| No estar de acuerdo con el revisor | I kept it as is — moving it would break the public API. Let me know if you still prefer the change |
Una revisión difícil, paso a paso
Escenario · 1 decisión como mínimo
Encontraste un problema serio en el pull request de alguien más senior que vos
Revisás un pull request de alguien con cinco años más de experiencia que vos, en un equipo donde se escribe todo en inglés. La consulta nueva no filtra por `tenantId`: cualquier cuenta podría leer datos de otra. Estás bastante seguro, pero no del todo: quizás el filtro está más arriba y no lo viste. ¿Qué escribís?
La persona responde: "The filter is applied in the repository layer, see line 40."
DesenlaceTenía el filtro y vos afirmaste que había un bug de seguridad. No pasó nada grave, pero la próxima vez vas a dudar antes de comentar, que es justo lo contrario de lo que un equipo necesita de un revisor. Preguntar en vez de afirmar te habría costado tres palabras.
La persona responde "good point, will look into it later" y mergea.
DesenlaceEl cambio salió a producción con la consulta sin filtrar. El comentario existía, pero nadie podía distinguirlo de una sugerencia de estilo. Suavizar el tono está bien; suavizar la severidad, no. Para eso está el prefijo `blocking:`.
La persona responde: "You're right, good catch — the filter was only in the other repository method." Sube un arreglo y te pide que lo mires. ¿Cómo cerrás?
El pull request se mergea sin más.
DesenlaceFuncionó. Lo que se perdió es barato de conseguir: una revisión que además de encontrar el problema deja el equipo mejor —un test de regresión, un reconocimiento— cuesta una línea más.
Se agrega el test y se mergea.
DesenlaceEl mejor final: el bug no salió, hay un test que impide que vuelva, y la relación con alguien más senior quedó mejor que antes. Los tres resultados salieron de la misma decisión inicial: describir el hecho, marcar que bloquea y dejar lugar a estar equivocado.
Notá que en ningún momento el comentario bueno es más largo que el malo. Es igual de largo y está mejor ordenado.
Practicalo
Reescribilo
Un comentario de revisión correcto en lo técnico y áspero en la forma. Reescribilo para que el autor pueda responder sin ponerse a la defensiva, sin perder precisión ni severidad.
this is wrong, you are doing a query inside the loop. you always do this. please fix it and also rename the variable, it is horribleblocking: this runs one query per item, so a 500-item order hits the DB 500 times. Could we fetch them all up front and look them up in a map? nit: `d` is a bit hard to follow — maybe `deliveryDate`?
Habla del código y no de la persona, explica la consecuencia con un número, propone una salida y separa lo que bloquea de lo que es un detalle. El «you always do this» desaparece: eso no es una revisión de código, es una conversación distinta.
question: is this query inside the loop intentional? With 500 items it would be 500 round trips — if not, fetching them up front would do it in one.
Cuando no estás seguro de que sea un error, preguntar primero cuesta lo mismo y te deja parado en mejor lugar si había un motivo.
Lo que no cambia entre el original y los modelos: el problema señalado es el mismo y sigue frenando el merge.
nit
/nɪt/ · suena como nit, corto
Error común: alargarlo como «niit»
merge
/mɜːdʒ/ · suena como merch, con ch suave
Error común: decir «merg» con g dura
approve
/əˈpruːv/ · suena como aprúuv
Error común: decir «apróv», acortando la u
blocking
/ˈblɒkɪŋ/ · suena como blóking
Error común: pronunciar la g final como en «tango»
threshold
/ˈθreʃhəʊld/ · suena como zréshold, con z de «think»
Error común: decir «tréshold» con t
El audio lo genera tu navegador con voz sintetizada: alcanza para orientarse, y una persona que habla inglés lo dice mejor.
Lo que preguntan sobre esto
En la práctica
Cierre
Autoevaluación
¿Lo entendiste?
Práctica