Comparación entre Unified Diff y Split Diff

• En cambios pequeños, tanto unified diff como split diff permiten revisar sin problema, pero en cambios grandes y complejos, una revisión basada solo en el diff llega a su límite
• Los cambios grandes deben explorarse y validarse tomando como base el estado completo del codebase en un momento dado, como si se estuviera modificando el código real, más que siguiendo el historial de cambios
• La pantalla de revisión ideal debería mostrar lado a lado el código actual en disco y el unified diff de esa zona, para seguir al mismo tiempo el contexto del código y los cambios
• Como el soporte en las herramientas existentes es insuficiente, se usa un flujo de trabajo de baja tecnología: hacer checkout local del PR, borrar los commits y dejar solo los cambios
• Con este método se puede recorrer los archivos modificados y marcar los hunks revisados, pero todavía queda trabajo manual porque la posición actual del editor y el diff no se sincronizan automáticamente

---

Revisar el estado actual del código, no solo el diff

• Tanto split diff como unified diff funcionan bien en cambios pequeños y simples
• Cuando los cambios crecen en tamaño y complejidad, ya no basta con leer el diff
• En cambios grandes, hay que inspeccionar correctamente el codebase en un momento determinado
  • Prestando atención a las zonas cambiadas recientemente, pero en esencia revisándolo como una revisión de código normal
  • Hay que ejecutar las pruebas
  • Hay que usar funciones de navegación del editor como goto definition
  • Hay que aplicar cambios locales para comprobar si había otra forma posible de escribirlo
  • Hay que buscar partes que debieron cambiar dentro de un contexto más amplio
  • Hay que poder detectar cosas extrañas en el codebase actual, sin depender del historial de cambios

La pantalla de revisión deseada y la alternativa local

• La vista ideal del diff tendría a la izquierda el estado actual del código y a la derecha el unified diff correspondiente al área de código visible en ese momento
  • El código de la izquierda debería ser igual al estado actual en disco
  • Los cambios se marcarían sutilmente en el margen
  • El diff de la derecha debería corresponder al área de código visible actualmente a la izquierda
• Hoy en día hay poco soporte de herramientas para este formato de revisión
• En la práctica, el flujo de trabajo consiste en hacer checkout local del PR con el script gpr
  • En el último paso, se borran los commits del PR y se dejan solo los cambios
  • En ese estado, se reutiliza para la revisión de código el flujo de trabajo de staging y commits
• Con edamagit se puede ver la lista de archivos modificados y moverse al cambio anterior o siguiente desde el editor
  • También se puede usar el área de staging para marcar los hunks ya revisados
• La incomodidad que queda es que no existe sincronización automática entre el magit status buffer y el archivo que está abierto actualmente en el editor
  • Para ver lado a lado el archivo actual y el diff, hay que abrir manualmente el diff y desplazarse por cuenta propia hasta la posición que se está viendo
• Si se quiere hacer una revisión tan cercana al código, por ahora toca construir herramientas temporales por cuenta propia
• El objetivo principal de una revisión de código no siempre es únicamente revisar el código; como lectura relacionada también se puede ver Two Kinds of Code Review

[GN⁺]

Opiniones de Hacker News

• Decir que, ante cambios grandes, no se quiere hacer una revisión de diff, sino revisar correctamente la base de código en un momento específico y prestar atención a las áreas modificadas recientemente, personalmente no me parece que encaje con una revisión de código común.
  Dependerá del equipo y del ticket, pero si el revisor no tiene un rol tipo principal engineer, responsable principalmente de todo el código, normalmente veo la revisión de código más como una simple verificación de coherencia hecha por un segundo par de ojos.
  Si los commits del PR están bien organizados o se hicieron squash, también es relativamente fácil revisar los cambios incrementales por separado.
  Al final, la queja parece más una diferencia en el tipo de revisión. Hay revisiones comunes de verificación de coherencia y revisiones profundas de arquitectura/funcionalidad antes del merge, y las revisiones de código en herramientas web parecen estar, en su mayoría, más cerca de lo primero.

  • Ver la revisión de código como una simple verificación de coherencia es una perspectiva demasiado limitada. La revisión de código es un mecanismo clave para reducir la propiedad individual del código, difundir convenciones y hacer transferencia de conocimiento técnico.
    Una buena revisión de código empieza con un buen PR. El PR debe explicar el objetivo y cómo se logró ese objetivo.
    El primer punto de una buena revisión es confirmar si el código cumple el objetivo descrito; el segundo, si hay pruebas que validen la funcionalidad declarada; el tercero, si sigue las convenciones arquitectónicas del sistema existente; el cuarto, si el estilo coincide con lo esperado; y, por último, si hay sugerencias para mejorar el uso de la API.
    Una revisión de código que solo hace verificación de coherencia aporta poco, porque eso lo podría hacer CI. La revisión de código es un proceso humano, así que hay que aprovechar al máximo lo que solo los humanos pueden hacer y dejar el resto a las máquinas.
    En los puntos anteriores está implícita la pregunta: “¿Este cambio será un buen ejemplo para futuras contribuciones?”.
  • Creo que las revisiones de código superficiales solo producen resultados superficiales. Esa es la razón por la que se despliegan tantos bugs a pesar de que las revisiones de código son habituales.
    Siempre he hecho revisiones de código desde una perspectiva de tester. Compilo y ejecuto el código para ver si funciona como se esperaba, busco puntos débiles e intento romperlo.
    El diff no es el final, sino el inicio. Solo es una pista que indica qué partes del código conviene probar; al final, la prueba viene del comportamiento real.
    Se podría decir “eso es trabajo de un tester, no de un ingeniero”, pero ¿quién podría probarlo mejor que la persona que entiende el código y sabe dónde mirar?
    Sé que esta actitud es minoritaria. Muchos ingenieros parecen tener una resistencia extrañamente fuerte a ejecutar el código ellos mismos, incluso a usar un depurador. Construyen infraestructuras enormes para automatizarlo todo, pero la automatización, al final, no es más que código que también puede tener bugs. ¿Quién prueba las pruebas? Quis custodiet ipsos custodes?
  • Estoy de acuerdo con lo que se espera de una revisión de código, pero para que funcione bien primero debe existir una forma de trabajo específica.
    La revisión de código es la última línea de defensa, salvo por CI, pero en los equipos donde trabajé, la gran dirección que introducía un PR ya se había discutido con varias personas antes de ese punto. Puede ser pairing, explicaciones profundas, charlas de café o cualquier método acorde con la complejidad.
    Entonces el PR se convierte en un espacio para revisar las tácticas que implementan esa estrategia, no la estrategia en sí, como separar un nuevo módulo, introducir una dependencia grande o rehacer la superficie de una API.
    Si se verifica la coherencia de solo una parte del código sin comunicación previa, existe el riesgo de fragmentar la propiedad del código. Se termina en una situación como: “¿Qué hace ese módulo?” “No sé, pregúntale a Bob”.
    Por separado, sin importar cuál sea el modo de revisión, me gusta la idea de vista de diff que propone el autor.
  • Hay algunas cosas buenas, pero cuando se experimenta repetidamente con la revisión de diff, muchas veces simplemente se vuelve incomprensible, así que pierde bastante sentido.
    Con bloques donde se mezclan líneas rojas y verdes es difícil hacer cualquier revisión. No es una vista útil del código y, al menos como vista predeterminada, no es adecuada.
    Aun así, como es parte del trabajo, uno la hace a la fuerza, pero estoy 100% de acuerdo con el autor en que no es una vista muy útil.
  • La cultura de las empresas tecnológicas modernas se ha alejado demasiado de procesos más formales para gestionar el desarrollo.
    Como no hay procesos básicos para manejar defectos, funcionalidades, etc., cada ingeniero tiene que inventar por su cuenta, una y otra vez, cómo desplegar una funcionalidad mediante colaboración.
    La mayoría termina haciendo casi nada, y la revisión de código pasa a ser la única colaboración que ocurre con un nivel de detalle significativo sobre el tema. Por eso se termina intentando meter todas las intervenciones posibles en un solo momento.

• Como tercera, o cuarta, opción, también vale la pena mencionar difftastic. Usa un diff “estructural” en lugar de uno por líneas, y resalta los cambios con más granularidad.
  https://github.com/Wilfred/difftastic

  • Como cuarta, o quinta, opción, también está patdiff: https://opensource.janestreet.com/patdiff/
    Según recuerdo, a veces hacía que alrededor del 35% de los diffs fueran más fáciles de leer; normalmente en un 60% no había diferencia, y en raras ocasiones, un 5%, los hacía más difíciles de leer.
    Lo usé hace algunos años, así que no recuerdo bien los problemas concretos. La única razón por la que dejé de usarlo fue que empecé a usar magit para los diffs de Git.
  • Es un enfoque realmente bueno. Lo lamentable es que no esté ampliamente integrado. Según entiendo, una salida JSON estructurada adecuada para que la usen otras herramientas apareció recién hace poco.
  • ediff de Emacs también hace esto. La función que resalta las palabras diferentes se llama Refine.
  • git diff --word-diff también tiene una función similar y es bastante buena.

• Al revisar con Vim se siente parecido
  Tengo un pequeño script que abre el PR y, para los archivos modificados, hace algo más o menos como vimdiff <(git show baseref:file) file. Las pestañas de Vim son, en la práctica, vistas, así que es bueno poder tener buffers individuales abiertos al mismo tiempo en distintos estados
  En la vista predeterminada, el bloqueo de desplazamiento funciona como se espera, y si hace falta se puede evitar en una pestaña aparte
  [c y ]c se mueven entre hunks tomando como referencia el conjunto de cambios que existe en el PR, no el directorio de trabajo
  Con dp y dg se puede hacer push/pull de un hunk hacia/desde el buffer de diff de solo lectura para marcarlo como “terminado”, y entonces se oculta de los cambios resaltados en el buffer vivo
  Los cambios hechos en el buffer vivo se pueden commitear de inmediato o pushear como comentario
  Las funciones normales del editor, como go to definition, integración de build y documentación en popups, también siguen funcionando tal cual sobre el estado actual del árbol
  Este enfoque permite ver los cambios del PR contra la rama base y, al mismo tiempo, mantener limpio el directorio de trabajo. Me parece mucho mejor que la solución de matklad, que ve los cambios dejando sucio el directorio de trabajo
  En el entorno donde trabajo esto es especialmente bueno porque los cambios a menudo se agregan como commits --fixup, y en el momento del merge la herramienta los vuelve a combinar, llamando a git-interpret-trailers para atribuir el autor del commit fixup al commit original. También extrae los comentarios de texto del PR y, si corresponde, agrega un trailer Reviewed-by, o un trailer Acked-by si equivale a un +1

  • Me da curiosidad qué significan exactamente “pushear como comentario” y “extrae los comentarios de texto”. ¿Es algún tipo de lógica personalizada interna de la empresa?

• En una base de código muy grande y compleja, con muchos ingenieros involucrados, esto es un gran problema
  La razón por la que el code review es difícil es que los diffs siempre parecen plausibles, las pruebas siempre pasan y lo básico siempre está revisado
  Pero aunque un cambio parezca razonable, muchas veces en realidad está mal
  Un solo cambio malo que parezca plausible puede hacer que toda la arquitectura derive
  Como siempre, esto no es estrictamente un problema de herramientas, sino más bien de cultura y de compartir conocimiento. No se resuelve solo con mejores herramientas

  • Me cuesta aceptar juntas las dos frases “las pruebas siempre pasan” y “aunque un cambio parezca razonable, muchas veces está mal”
    Al hacer code review rara vez se busca el bug en sí. Más bien se revisa si existen pruebas capaces de detectar ese tipo de bug

• Siento que me estoy perdiendo algo. El autor dice que el split diff no le sirve, pero no explica por qué
  Un diff ideal se vería casi igual que un split diff sin contexto duplicado. En vez de tener el contexto duplicado en ambos lados, estaría solo a la izquierda
  Me pregunto qué utilidad tiene quitar el contexto duplicado del panel derecho

  • El autor escribe que necesita ejecutar pruebas, usar go to definition y otras funciones de navegación del editor, aplicar cambios locales para comprobar si se pudo haber hecho de otra manera, ver un contexto más amplio para encontrar partes que también deberían haberse cambiado, y notar cosas raras en la base de código actual independientemente del camino histórico de los cambios
    Los editores/IDE que he usado normalmente tienen soporte básico para estas funciones dentro del diff. Go to definition solo funciona dentro del mismo archivo, o el autocompletado no funciona bien con elementos recién agregados, y muchas veces no hay funciones de refactorización
  • La izquierda muestra el código actual sin modificaciones. Si en un lado tienes el contexto completo con funciones de navegación de código y en el otro los cambios, puedes ver qué cambio se aplica en un contexto específico
    Según lo entiendo, el autor quiere invertir el diff. En vez de mirar el cambio en sí, quiere mirar el código y verificar si hay cambios relacionados ahí
    Al artículo le faltan detalles, pero parece que quiere navegar hacia la implementación o los puntos de llamada y comprobar si hay cambios adecuados
  • No soy el autor, pero me identifico
    El resaltado de diff puede distraer más de lo que uno cree, y a veces desactivarlo mejora claramente la legibilidad. Yo también lo desactivo a veces en el visor de diffs para ver el código con ojos frescos, pero no existe la opción de mostrar a la vez un diff unificado en un split pane

• Si presionas . en GitHub, entras a un IDE completo dentro del navegador
  Me resultó muy útil al revisar. Como puedes ver los cambios dentro del contexto completo del archivo, en vez de solo fragmentos, detectas mucho mejor problemas sutiles de diseño

  • Esto es buenísimo. Solo había entrado a este IDE al editar archivos directamente en GitHub; no sabía que existía un atajo para ir ahí desde el diff de un commit
    Como referencia, este atajo también funciona en GitLab

• Extraño varias funciones de p4merge. Por ejemplo, tiene una función que muestra en el espacio entre los split diffs qué parte de un archivo corresponde a qué parte del otro, y al mismo tiempo muestra el impacto del diff en ambos lados: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
  También me gusta la función de 3-way merge: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
  Estas funciones no han llegado a las herramientas web de diff más usadas. Creo que un 3-way diff es una buena forma de mostrar el resultado de un merge difícil

• Si usas GitHub, en la página de pull request de github.com puedes presionar . o cambiar el dominio a github.dev
  Entonces se abre VSCode en el navegador y el pull request se muestra en una vista de diff
  La ventaja es que puedes ver el archivo completo y, como usa un algoritmo de diff distinto al de github.com, a veces resulta más legible en diffs complejos

  • La desventaja es que es lento y tiene muchos bugs
    La vista de diff side-by-side que muestra los comentarios del PR como una lista fuera de las líneas podría ser muy útil, pero es una lástima tener que levantar una instancia completa de VSCode en el navegador
    De hecho era tan útil que GitHub antes tenía una split view así, y no hacía falta empujar a la gente a una maraña de buzzwords como entornos de desarrollo AI/ML/crypto/blockchain/cloud-enabled
    La idea es buena, pero la ejecución es pésima. También parece una buena forma de resumir GitHub en general últimamente

• Meld se adapta bastante bien a este uso
  https://meldmerge.org/

  • https://kdiff3.sourceforge.net/ también tiene muchas vistas buenas, así que vale la pena mencionarlo
  • Se ve bastante parecido a FileMerge, incluido en las herramientas para desarrolladores de Apple

• Mi herramienta favorita es diff2html-cli: https://diff2html.xyz/
  https://www.npmjs.com/package/diff2html-cli
  Permite ver diffs en HTML y admite formatos side-by-side o unified