Hay un montón de información en Internet sobre las revisiones de código:
- cómo una revisión del código de otra persona afecta la cultura de la empresa
- Verificaciones de seguridad regulatorias
- manuales resumidos
- largas listas de recomendaciones
- revisión de código desde una perspectiva interpersonal
- ¿Por qué necesito una revisión de código?
- métodos probados
- Más sobre métodos probados
- estadísticas: cuánto ayuda la revisión de código para detectar errores
- ejemplos de errores cometidos durante la revisión del código
Sí, por supuesto, hay
libros sobre este tema. En una palabra, este artículo describe cómo Palantir organiza la revisión del código. En aquellas organizaciones cuya cultura no acepta una revisión por pares, puede ser útil leer primero el brillante ensayo de Karl E. Wiegers, The Code Revision
with a Human Face, y luego intentar seguir esta guía.
Este texto está tomado de
las recomendaciones de mejora de calidad basadas en el trabajo con
Baseline , nuestra herramienta para el control de calidad del código Java. Cubre los siguientes temas:
- Por qué, qué y cuándo intentamos lograr la revisión del código
- Preparando código para revisión
- Ejecución de revisión de código
- Ejemplos de revisión de código
Traducido a Alconost 
XKCD Comic 'Code Quality', copiado bajo licencia CC BY-NC 2.5Motivación
Nos dedicamos a la revisión de código (RC) para mejorar la calidad del código y, por lo tanto, queremos
influir positivamente en el equipo y la cultura corporativa. Por ejemplo:
- Los autores del código están motivados porque saben que un equipo de revisión considerará su solicitud de cambios: el autor intenta limpiar la aspereza, seguir el plan de acción y, en general, mejorar todo el compromiso. Cuando los colegas reconocen su profesionalismo al escribir código, para muchos programadores, esta es una ocasión para estar orgullosos de sí mismos.
- Compartir conocimiento ayuda al equipo de desarrollo de varias maneras:
- Bajo el RK, se informa claramente qué parte de la funcionalidad se ha agregado / cambiado / eliminado para que en el futuro los miembros del equipo puedan confiar en el trabajo ya realizado.
- El autor del código puede usar una técnica o algoritmo en el cual los propios revisores aprenderán algo. En general, una revisión de código ayuda a
elevar el estándar de calidad en toda la organización .
- Los revisores pueden saber algo sobre las técnicas de programación o la base de código en sí misma que ayudará a mejorar o consolidar los cambios realizados; por ejemplo, alguien puede al mismo tiempo desarrollar o modificar exactamente la misma oportunidad.
- El contacto positivo y la comunicación refuerzan los lazos sociales entre los miembros del equipo.
- Debido a la coherencia en la base del código, el código en sí es mucho más fácil de leer y comprender. Es más fácil evitar errores y promover la colaboración entre programadores establecidos y nómadas.
- Es difícil para el autor del código juzgar la legibilidad de su propia creación, y para el revisor es simplemente fácil, porque no ve en qué contexto se encuentra este código. El código legible por humanos es más fácil de reutilizar, tiene menos errores y es más duradero.
- Los errores aleatorios (por ejemplo, errores tipográficos), así como los errores estructurales (por ejemplo, código no ejecutable, errores de lógica o algoritmos, problemas de rendimiento y arquitectura) a menudo son mucho más fáciles de detectar por un crítico exigente que mira el código desde un lado. Según la investigación, incluso una revisión breve y no oficial del código afecta significativamente la calidad del código y la aparición de errores .
- Los requisitos de cumplimiento y los marcos regulatorios a menudo requieren revisiones obligatorias. Una revisión de código es una excelente manera de evitar problemas de seguridad comunes. Si se aumentan los requisitos de seguridad en este entorno o en relación con esta oportunidad, los sátrapas de las autoridades reguladoras recomendarán (o incluso solicitarán) la revisión (un buen ejemplo es la guía OWASP ).
Que buscar
No hay una respuesta clara a esta pregunta, y cada equipo de desarrollo debe acordar su propio enfoque. Algunos equipos prefieren verificar los cambios realizados en la rama de desarrollo principal, mientras que otros tienen en cuenta el "umbral de trivialidad", más allá del cual la verificación es obligatoria. En este caso, es necesario lograr un equilibrio entre el uso eficiente del tiempo del programador (tanto el autor como el revisor del código) y mantener la calidad del código. En algunos contextos estrictos, a veces se requiere verificar todo, incluso los cambios más triviales, en el código.
Las reglas de revisión de código son las mismas para todos: incluso si usted es el desarrollador más veterano del equipo, esto no significa que su código no requiera una revisión. Incluso si (a veces sucede) el código es perfecto, la revisión abre oportunidades para la tutoría y la cooperación, y, al menos, ayuda a ver la base del código desde diferentes puntos de vista.
Cuando verificar
Se debe realizar una revisión del código después de completar con éxito las comprobaciones automáticas (pruebas, diseño, otras etapas de integración continua), pero antes de la fusión del nuevo código con la rama del repositorio principal. Por lo general, no recurrimos a la verificación formal de los cambios totales realizados en el código desde la última versión.
Para los cambios complejos que deben agregarse a la rama principal del código como una sola unidad, pero tan extensos que no se pueden cubrir en una sola comprobación, intente una revisión por fases. Creamos la rama de función principal / función grande y una serie de secundarias (por ejemplo, función / API de función grande, prueba de función / función grande, etc.), cada una de las cuales encapsula un subconjunto de la funcionalidad común, y cada una de esas ramas comprobado contra la rama principal de feature / big-feature. Después de que todas las ramas secundarias se fusionen en función / función grande, cree una revisión de código para inyectar la última en la rama principal.
Preparando código para revisión
El autor del código está obligado a proporcionar el código para la revisión en forma digerible, a fin de no tomar tiempo adicional de los revisores y no desmotivarlos:
- Alcance y tamaño . Los cambios deben relacionarse con un alcance estrecho, bien definido y autosuficiente, totalmente cubierto por la revisión. Por ejemplo, los cambios pueden estar relacionados con la implementación de alguna función o corrección de errores. Los cambios breves son preferibles a los largos. Si la revisión contiene material que incluye cambios en más de ~ 5 archivos, o requiere más de 1-2 días para grabar, o la revisión en sí misma puede tomar más de 20 minutos, intente dividir el material en varios fragmentos autocontenidos y verifique cada uno por separado. Por ejemplo, un desarrollador puede enviar un cambio que defina la API para una nueva característica en términos de interfaces y documentación, y luego agregar un segundo cambio que describa las implementaciones para estas interfaces.
- Debe enviar solo materiales completos, probados independientemente (use diff) y probados independientemente . Para ahorrar tiempo al revisor, pruebe los cambios enviados (es decir, ejecute el conjunto de pruebas) y asegúrese de que el código pase todos los ensamblajes, así como todas las pruebas y todas las etapas de control de calidad, tanto localmente como en servidores de integración continua, y solo luego seleccione revisores
- Los cambios de refactorización no deberían afectar el comportamiento y viceversa; Los cambios que afectan el comportamiento no deben implicar refactorizar o formatear el código. Hay una serie de buenas razones para esto:
- Los cambios relacionados con la refactorización generalmente afectan muchas líneas de código en muchos archivos, por lo tanto, este código
no se
verificará tan cuidadosamente . Los cambios no planificados en el comportamiento pueden filtrarse en la base del código, y nadie lo notará.
- Los cambios importantes asociados con la refactorización violan los mecanismos de movimiento, selección y otra "magia" asociada con el control de la fuente. Es muy difícil deshacer ese cambio de comportamiento que se realizó después de completar la refactorización que cubrió todo el repositorio.
- El costoso tiempo de trabajo que una persona dedica a revisar el código debe ir a verificar la lógica del programa y no a disputas sobre el estilo, la sintaxis o el formato del código. Preferimos resolver estos problemas utilizando herramientas automatizadas, en particular,
Checkstyle ,
TSLint ,
Baseline ,
Prettier , etc.
Confirmar mensajes
El siguiente es un ejemplo de un mensaje de confirmación competente, que está diseñado de acuerdo con el estándar ampliamente utilizado:
( 80 ) , , . 72 . , — . , ( ); , rebase, . - : « », « » « ». -, , , git merge git revert. . .
Intente describir los cambios realizados durante la confirmación y cómo se realizan exactamente estos cambios:
> . . . > . jcsv- IntelliJ
Buscar revisores
Por lo general, el autor de una confirmación busca uno o dos revisores que estén familiarizados con la base del código. A menudo en esta capacidad son el gerente de proyecto o ingeniero senior. Es recomendable que el propietario del proyecto se suscriba a su propio proyecto para recibir notificaciones de nuevas revisiones de código. Con la participación de tres o más revisores, una revisión de código a menudo resulta improductiva o contraproducente, ya que diferentes revisores pueden proponer cambios mutuamente conflictivos. Esto puede indicar un desacuerdo fundamental sobre la implementación correcta, y tales problemas no deben resolverse durante la revisión del código, sino en una reunión extendida en la que todas las partes interesadas participen en persona o en un modo de videoconferencia.
Ejecución de revisión de código
Una revisión de código es un punto de sincronización entre diferentes miembros del equipo, por lo que potencialmente puede detener el trabajo. Por lo tanto, la revisión del código debe estar operativa (tomar varias horas, no días), y los miembros del equipo y los líderes deben saber cuánto tiempo tomará verificar y priorizar el tiempo asignado en consecuencia. Si le parece que no tiene tiempo para terminar la revisión a tiempo, informe inmediatamente al autor del compromiso para que pueda encontrar un reemplazo para usted.
La revisión debe ser bastante exhaustiva para que el revisor pueda explicar el cambio a otro desarrollador con suficiente detalle. Por lo tanto, nos aseguraremos de que los detalles de la base del código sean conocidos por al menos dos, y no por uno.
Usted, como revisor, debe cumplir con los estándares de calidad del código y mantenerlo en un alto nivel. Una revisión de código es más un arte que una ciencia. Esto solo se puede aprender en la práctica. Un revisor experimentado debe intentar primero dejar que los colegas menos experimentados cambien y dejar que sean los primeros en revisar. Si el autor siguió las reglas anteriores (especialmente las relacionadas con la autocomprobación y la ejecución preliminar del código), entonces esto es a lo que el revisor debe prestar atención al revisar:
Propósito
- ¿El código logra el objetivo establecido por el autor? Cualquier cambio debe tener un motivo específico (nueva función, refactorización, corrección de errores, etc.). ¿El código propuesto realmente nos lleva a este objetivo?
- Haz preguntas Las funciones y clases deben estar justificadas. Si su asignación al revisor no está clara, tal vez esto significa que el código debe ser reescrito o respaldado por comentarios o pruebas.
Implementación
- Piensa en cómo resolverías este problema. Si no, ¿por qué? ¿Su código maneja más casos (límite)? ¿Quizás es más corto / más fácil / más limpio / más rápido / más seguro y funcionalmente no es peor? ¿Quizás captó alguna regularidad profunda que no está cubierta por el código existente?
- ¿Ves el potencial para crear abstracciones útiles? Si el código está parcialmente duplicado, esto a menudo significa que se puede extraer de él un elemento más abstracto o general del funcional, que luego se puede reutilizar en otros contextos.
- Piensa como un oponente, sin embargo, mantente agradable. Intente "atrapar" a los autores que cortan esquinas o pierden casos específicos lanzando configuraciones problemáticas / datos de entrada que rompen su código.
- Piense en bibliotecas o código de trabajo listo para usar. Si alguien vuelve a implementar la funcionalidad existente, esto no se debe solo a que no conoce la existencia de una solución preparada. A veces, el código o la funcionalidad se reinventa intencionalmente, por ejemplo, para evitar dependencias. En este caso, esto debería comentarse claramente en el código. ¿Esta funcionalidad ya está disponible en alguna biblioteca existente?
- ¿El cambio se ajusta a los patrones estándar? En las bases de código existentes, las regularidades a menudo se rastrean en relación con las convenciones de nomenclatura, la descomposición de la lógica del programa, las definiciones de los tipos de datos, etc. Por lo general, es deseable que todos los cambios se implementen de acuerdo con los patrones existentes.
- ¿Se agregan dependencias que ocurren durante la compilación o en tiempo de ejecución (especialmente entre subproyectos) durante un cambio? Nos esforzamos por la débil coherencia del código de nuestros productos, es decir, tratamos de permitir un mínimo de dependencias. Los cambios relacionados con las dependencias y el sistema de compilación deben verificarse con especial cuidado.
Legibilidad y estilo
- Piensa en cómo lees este código. ¿Puedes comprender sus conceptos lo suficientemente rápido? ¿Está razonablemente establecido, es fácil hacer un seguimiento de los nombres de variables y métodos? ¿Puedo rastrear código en múltiples archivos o funciones? ¿Alguna vez te han molestado los nombres inconsistentes en alguna parte?
- ¿El código cumple con las pautas de programación y estilo bien conocidas? ¿Se está rompiendo el código de todo el proyecto por estilo, convenciones de nomenclatura de API, etc.? Como se mencionó anteriormente, tratamos de resolver disputas estilísticas utilizando herramientas automáticas.
- ¿El código tiene listas de tareas (TODOs)? Las listas de tareas simplemente se acumulan en el código y eventualmente se convierten en lastre. Asigne al autor para que envíe un ticket a GitHub Issues o JIRA y adjunte un número de tarea a TODO. No debe haber código comentado en el cambio propuesto.
Soporte de usabilidad
- Lee las pruebas. Si no hay pruebas en el código, pero deberían estar allí, pídale al autor que escriba algunas. Las características verdaderamente no probadas son raras, y no implementaciones de características probadas, desafortunadamente, a menudo. Verifique las pruebas en sí mismas: ¿cubren casos interesantes? ¿Es conveniente leerlos? ¿La cobertura general de la prueba disminuye después de esta prueba? Piensa en cómo podría fallar este código. Los estándares de diseño de prueba y el código central a menudo son diferentes, pero los estándares de prueba también son importantes.
- ¿La revisión de este fragmento presenta un riesgo de romper el código de prueba, el código de intrusión o las pruebas de integración? A menudo, dicha revisión no se realiza antes de la confirmación / fusión, pero si se descuidan tales casos, todos sufrirán. Preste atención a las siguientes cosas: eliminar utilidades o modos de prueba, cambiar la configuración, cambios en el diseño / estructura del artefacto.
- ¿Este cambio viola la compatibilidad con versiones anteriores? Si es así, ¿es posible realizar este cambio en la versión en esta etapa, o debería posponerse para una versión posterior? Las violaciones pueden incluir cambios en la base de datos o su esquema, cambios en las API públicas, cambios en el flujo de tareas a nivel de usuario, etc.
- ¿Este código requiere pruebas de integración? A veces, el código no se verifica correctamente utilizando solo pruebas unitarias, especialmente si interactúa con sistemas o configuración externos.
- Deje comentarios sobre la documentación de este código, comentarios y mensajes de confirmación. Los comentarios extensos obstruyen el código y los mensajes de confirmación confusos confunden a aquellos que luego tienen que trabajar con el código. Este no es siempre el caso, pero los comentarios de calidad y los mensajes de confirmación a lo largo del tiempo sin duda le servirán. (Recuerde cuando vio un comentario o mensaje de confirmación magnífico o simplemente horrible).
- ¿Se actualizó la documentación externa? Si se mantiene README, CHANGELOG u otra documentación para su proyecto, ¿se actualiza para reflejar los cambios realizados? La documentación obsoleta puede ser más dañina que ninguna, y será más costoso corregir errores en el futuro que hacer cambios ahora.
No te olvides de alabar el código conciso / legible / eficiente / hermoso. Por el contrario, rechazar o rechazar un código propuesto para revisión no es grosero. Si el cambio es excesivo o insignificante, rechazarlo, explicando la razón. Si el cambio le parece inaceptable debido a uno o varios errores fatales, vuelva a rechazarlo con razón. A veces, el mejor resultado de una revisión de código es decir: "Hagámoslo de manera completamente diferente" o "No lo hagamos en absoluto".
Respeto revisado por pares. Aunque la posición del oponente es sólida, no te diste cuenta de esta oportunidad y no puedes decidir todo por él. Si no puede llegar a una opinión revisada por pares sobre el código, organice una consulta en tiempo real y escuche la opinión de una tercera persona.
Seguridad
Asegúrese de que todos los puntos finales API estén debidamente autorizados y autenticados de acuerdo con las reglas adoptadas en el resto de la base de código. Busque otros defectos comunes, por ejemplo: configuración débil, entrada maliciosa del usuario, entradas de registro faltantes, etc. En caso de duda, muestre el código revisado por pares a un profesional de seguridad.
Comentarios: conciso, cortés, incentivo
La revisión debe ser concisa, sostenida en un estilo neutral. Critica el código, no el autor. Si algo no está claro, solicite una aclaración y no asuma que todo está en la ignorancia del autor. Evite los pronombres posesivos, especialmente en los juicios de valor: "
mi código funcionó antes de
sus cambios", "hay un error en
su método", etc. No corte el hombro: "
no funcionará
en absoluto ", "el resultado
siempre es incorrecto".
Trate de distinguir entre las recomendaciones (por ejemplo, "Recomendación: extraer el método, esto aclarará el código"), los cambios necesarios (por ejemplo, "Agregar
anulación ") y las opiniones que necesitan aclaración o discusión (por ejemplo, "¿Es cierto este comportamiento? Si es así, por favor agregue un comentario explicando la lógica ".) Intente poner enlaces o punteros a una descripción detallada del problema.
Cuando finalice la revisión del código, indique qué tan detallada es la reacción a sus comentarios que espera del autor, le gustaría repetir la revisión después de realizar los cambios, por ejemplo: "Puede cargar el código de forma segura en la rama principal después de estas correcciones menores" o "Tenga en cuenta mis comentarios y avíseme cuando pueda ver el código nuevamente ".
Respuesta de revisión
La revisión del código, en particular, está destinada a mejorar la solicitud de cambios propuesta por el autor; por lo tanto, no tome la hostilidad de las recomendaciones del revisor, tómelas en serio, incluso si no está de acuerdo con ellas. Responda a cualquier comentario, incluso al habitual "ACK" (aprobado) o "hecho" (hecho). Explique por qué tomó esta o aquella decisión, por qué tiene ciertas funciones en su código, etc. Si no puede llegar a una opinión común con el revisor, organice una consulta en tiempo real y escuche la opinión de un tercero especialista.
Las correcciones deben corregirse en la misma rama, pero en una confirmación por separado. Si mantiene las confirmaciones en la etapa de revisión, será más difícil para el revisor realizar un seguimiento de los cambios.
La política de fusión de código es diferente para diferentes equipos. En algunas empresas, el propietario del proyecto solo confía en la fusión de códigos; en otras, un participante puede hacerlo después de que su código haya sido revisado y aprobado.
Revisión del código Tête-à-tête
Como regla, las herramientas asíncronas de tipo diff, como Reviewable, Gerrit o GitHub, son excelentes para las revisiones de código. Si hablamos de una revisión compleja o una revisión, cuyos participantes difieren mucho en su nivel de experiencia o profesionalismo, puede ser más efectivo realizar una revisión en persona, ya sea sentados juntos frente a un monitor o proyector, o de forma remota, mediante videoconferencia o herramientas para compartir pantalla.
Ejemplos
En los siguientes ejemplos, los comentarios de los revisores en los listados se ingresan usando
Nomenclatura inconsistente
class MyClass { private int countTotalPageVisits;
Firmas de métodos inconsistentes
interface MyInterface { public Optional<String> extractString(String s);
Uso de la biblioteca
//R: MapJoiner Guava String joinAndConcatenate(Map<String, String> map, String keyValueSeparator, String keySeparator);
Preferencias personales
int dayCount;
Bichos
//R: numIterations+1 – ? // – numIterations? for (int i = 0; i <= numIterations; ++i) { ... }
Consideraciones arquitectónicas
otherService.call();
Sobre el traductorEl artículo fue traducido por Alconost.
Alconost
localiza juegos ,
aplicaciones y sitios en 70 idiomas. Traductores en lengua nativa, pruebas lingüísticas, plataforma en la nube con API, localización continua, gestores de proyectos 24/7, cualquier formato de recursos de cadena.
También hacemos
videos de publicidad y capacitación , para sitios que venden, imágenes, publicidad, capacitación, teasers, expliner, trailers de Google Play y App Store.
Más detalles