De la documentación de Prácticas de ingeniería de Google

Esta guía proporciona las mejores prácticas para realizar revisiones de códigos basadas en años de experiencia. Juntos, forman un documento, dividido en muchas secciones. No es necesario leerlos todos, pero a menudo es mejor para usted y el equipo estudiar el manual en su totalidad.

• Norma de revisión de código
  
• Qué verificar en el código
  
• Cambiar lista de navegación (CL)
  
• Velocidad de revisión de código
  
• Cómo escribir comentarios
  
• Cómo vencer la resistencia

Consulte también la Guía del autor de CL para obtener consejos detallados sobre los desarrolladores cuyas confirmaciones se revisan.

Norma de revisión de código

El objetivo principal de una revisión de código es garantizar la mejora continua de la base de código de Google. Todas las herramientas y procesos están dedicados a este objetivo.

Aquí se necesitan varios compromisos.

Primero, los desarrolladores deberían poder resolver con éxito sus problemas . Si nunca envía un código, la base del código nunca mejorará. Además, si el revisor complica mucho cualquier trabajo, en el futuro, los desarrolladores no están interesados ​​en proponer mejoras.

Por otro lado, es responsabilidad del revisor asegurarse de que la calidad del CL no reduzca la calidad general de la base del código con el tiempo. Esto puede ser difícil, porque a menudo se produce degradación debido a una ligera disminución en la calidad del código con el tiempo, especialmente si el equipo está bajo una fuerte presión por los plazos y siente que tiene derecho a aumentar la deuda técnica.

Además, el revisor es responsable del código que se está revisando. Quiere asegurarse de que la base del código permanezca consistente, compatible y coincida con todo lo demás que se menciona en la sección "Qué verificar en el código" .

Por lo tanto, obtenemos la siguiente regla como estándar para la revisión de código:

Por lo general, los revisores deben respaldar el CL tan pronto como llegue a un estado en el que definitivamente mejore la calidad general del código del sistema, incluso si el CL no es perfecto.

Esta es la revisión del código principal entre todos los principios.

Por supuesto, tiene limitaciones. Por ejemplo, si CL agrega una función que el revisor no quiere ver en el sistema, entonces el revisor puede negarse a comprometerse, incluso si el código es de buena calidad.

El punto clave aquí es que no hay un código "perfecto", solo hay un código mejor . El revisor no debe exigir al autor que pule cada pequeño fragmento. Por el contrario, el revisor debe equilibrar la necesidad de un mayor progreso sobre la importancia de los cambios propuestos. En lugar de luchar por el ideal, el revisor debe luchar por la mejora continua . Un compromiso, que generalmente mejora la capacidad de mantenimiento, la legibilidad y la comprensión del sistema, no se puede retrasar durante días o semanas porque no es "perfecto".

Los revisores siempre pueden dejar comentarios sobre la mejora del código, pero los cambios no muy importantes deben marcarse, por ejemplo, con el prefijo Nit: para que el autor sepa que este es solo un punto de vista que puede ignorar.

Nota Nada en este documento justifica los CL que definitivamente degradan la calidad general del código del sistema. Esto solo es posible en una emergencia .

Mentoring

Una revisión de código también puede ser importante para enseñar a los desarrolladores algo nuevo sobre el lenguaje, la estructura o los principios generales del diseño de software. Siempre es bueno dejar comentarios que ayudan al desarrollador a aprender algo nuevo. Compartir conocimiento contribuye a mejorar el código del sistema con el tiempo. Solo tenga en cuenta que si deja un comentario puramente educativo que no es crítico para cumplir con los estándares descritos aquí, agregue el prefijo Nit: o indique que el autor no está obligado a permitirlo.

Principios

• Los datos y datos técnicos superan las opiniones y preferencias personales.
  
• En materia de estilo, la autoridad absoluta es la guía del estilo . Cualquier detalle puramente estilístico (espacio, etc.) que no esté incluido en la guía de estilo es una cuestión de preferencia personal. El estilo debe coincidir con lo que es. Si no hay un estilo anterior, acepte al autor.
  
• Los aspectos del diseño de software casi nunca son cuestión de puro estilo o preferencia personal. Se basan en principios fundamentales y deben estar determinados por estos principios, y no solo por la opinión personal. A veces hay varias opciones válidas. Si el autor puede demostrar (ya sea utilizando datos o basándose en principios de ingeniería sólidos) que ciertos enfoques son igualmente efectivos, el revisor debe aceptar la preferencia del autor. De lo contrario, la elección está dictada por los principios de desarrollo estándar.
  
• Si no se aplica ninguna otra regla, el revisor puede pedirle al autor que observe la uniformidad con la base del código actual, si esto no empeora la condición general del sistema.

Resolución de conflictos

En cualquier conflicto, el primer paso siempre debe ser el deseo del desarrollador y el revisor de llegar a un consenso basado en el contenido de este documento y otros documentos en la Guía del autor de CL y esta Guía del revisor .

Cuando es especialmente difícil llegar a un consenso, una reunión personal o una videoconferencia entre el revisor y el autor puede ayudar (si lo hace, asegúrese de anotar los resultados de la discusión en un comentario al compromiso para futuros lectores).

Si esto no resuelve la situación, la forma más común es la escalada. A menudo consiste en una discusión más amplia con el equipo, atraer al líder del equipo, contactar al encargado de mantenimiento o al gerente de desarrollo. No permita que el compromiso se demore debido al hecho de que el autor y el revisor no pueden llegar a un acuerdo.

Qué verificar en el código

Nota Al considerar cada uno de estos elementos, asegúrese de considerar la Revisión del Código Estándar .

Diseño

Lo más importante es considerar el proyecto general (diseño) en la revisión del código. ¿Tienen sentido las interacciones entre diferentes partes del código? ¿Este cambio se aplica a su base de código o biblioteca? ¿CL se integra bien con el resto del sistema? ¿Es hora de agregar esta funcionalidad ahora?

Funcionalidad

¿Este CL hace lo que el desarrollador pretendía? ¿Es bueno para los usuarios de este código? Por "usuarios" nos referimos tanto a los usuarios finales (si se ven afectados por el cambio) como a los desarrolladores (que tendrán que "usar" este código en el futuro).

Básicamente, esperamos que incluso antes de comprometerse, los desarrolladores prueben su código lo suficientemente bien como para que funcione correctamente. Pero como revisor, aún debe pensar en casos extremos, buscar problemas de concurrencia, tratar de pensar como usuario e incluso observar mientras lee el código que no hay errores obvios.

Si lo desea, puede verificar el rendimiento. Esto es más importante si el código afecta a los usuarios, como cambiar la IU . Es difícil entender cómo algunos cambios afectarán a los usuarios cuando solo lea el código. Para tales cambios, puede pedirle al desarrollador que proporcione una demostración si le resulta demasiado difícil profundizar en el código y probarlo usted mismo.

Otro punto en el que es especialmente importante pensar en la funcionalidad durante una revisión de código es si se produce algún tipo de programación paralela en el CL, que teóricamente puede causar puntos muertos o condiciones de carrera. Tales problemas son muy difíciles de detectar simplemente ejecutando el código; por lo general, es necesario que alguien (tanto el desarrollador como el revisor) lo piense detenidamente y se asegure de que no se presenten problemas (tenga en cuenta que esta también es una buena razón para no usar modelos de paralelismo donde las condiciones de carrera o los puntos muertos son posibles; esto puede hacerse por código muy difícil de entender o revisión de código).

Dificultad

¿Es CL más complicado de lo que debería ser? Verifíquelo en todos los niveles: líneas separadas, funciones, clases. "Complejidad excesiva" generalmente significa la incapacidad de comprender rápidamente mientras lee . También puede significar que es más probable que los desarrolladores introduzcan errores al intentar llamar o modificar este código .

Un tipo especial de complejidad es la ingeniería excesiva, cuando los desarrolladores han hecho que el código sea más universal de lo que debería ser, o agregan funcionalidades que el sistema no necesita actualmente. Los revisores deben estar especialmente atentos a la sobre ingeniería. Aliente a los desarrolladores a resolver un problema que definitivamente debería resolverse ahora, en lugar de un problema que deba resolverse en el futuro. Un problema futuro debe resolverse cuando aparezca, y puede ver su forma real y sus requisitos en el Universo físico.

Pruebas

Solicite pruebas de unidad, integración o de extremo a extremo que sean relevantes para el cambio. En general, las pruebas deben agregarse al mismo CL que el código de producción si el CL no maneja la emergencia .

Asegúrese de que las pruebas sean correctas, razonables y útiles. Las pruebas no se evalúan por sí mismas, y rara vez escribimos pruebas para nuestras pruebas; una persona debe asegurarse de que las pruebas sean válidas.

¿Las pruebas realmente fallan en el código roto? Si el código cambia, ¿habrá falsos positivos? ¿Cada prueba hace declaraciones simples y útiles? ¿Están las pruebas correctamente divididas entre diferentes métodos de prueba?

Recuerde que las pruebas son códigos que también deben ser compatibles. No permita la complejidad en ellos solo porque no es parte del archivo binario principal.

Nombrar

¿El desarrollador ha elegido buenos nombres en todas partes? Un buen nombre es lo suficientemente largo como para transmitir completamente qué es el elemento o qué hace, sin ser tan largo que se vuelva difícil de leer.

Comentarios

¿El desarrollador escribió comentarios claros en lenguaje sencillo? ¿Son necesarios todos los comentarios? Los comentarios suelen ser útiles cuando explican por qué existe algún código, y no deberían explicar qué hace este código. Si el código no es lo suficientemente claro como para explicarse, entonces debería simplificarse. Hay algunas excepciones (por ejemplo, los comentarios que explican las acciones del código a menudo son muy útiles para expresiones regulares y algoritmos complejos), pero la mayoría de los comentarios están destinados a información que el código en sí no puede contener, por ejemplo, para justificar una decisión.

También puede ser útil mirar los comentarios en el código anterior. Quizás haya un TODO, que ahora se puede eliminar, o un comentario que no recomiende introducir este cambio, etc.

Tenga en cuenta que los comentarios difieren de la documentación para clases, módulos o funciones, que describe la tarea del código, cómo debe usarse y cómo se comporta.

Estilo

Tenemos guías de estilo de Google para todos los idiomas principales, e incluso la mayoría de los idiomas menores. Asegúrese de que el CL no esté en conflicto con las guías de estilo relevantes.

Si desea mejorar algunos elementos que no están en la guía de estilo, agregue una nota al comentario ( Nit :) . El desarrollador sabrá que este es su comentario personal, que no es vinculante. No bloquee el envío de confirmaciones basadas únicamente en sus preferencias personales.

El autor no debe combinar cambios de estilo significativos con otros cambios. Esto hace que sea difícil ver cambios en la CL, complica las fusiones, las reversiones de código y causa otros problemas. Por ejemplo, si el autor desea volver a formatear todo el archivo, solicite un cambio de estilo en un CL y luego envíe el CL con cambios funcionales.

La documentación

Si la salida de CL cambia algo en el ensamblaje, las pruebas, los procedimientos de interacción o la publicación del código, verifique las actualizaciones de la documentación relevante, incluidos los archivos README, las páginas g3doc y todos los documentos de referencia generados. Si el CL elimina el código o lo vuelve obsoleto, considere si también debe eliminar la documentación. Si falta la documentación, solicite su creación.

Cada fila

Ver cada línea en el código. Si bien los archivos de datos, el código generado o las estructuras de datos grandes se pueden revisar brevemente, lea cuidadosamente cada clase, función o bloque de código escrito por una persona, nunca asuma por defecto que todo está en orden. Obviamente, algunos códigos merecen un estudio más exhaustivo que otros: usted decide por sí mismo, pero al menos debe asegurarse de comprender el funcionamiento de todo el código.

Si le resulta demasiado difícil leer el código y esto ralentiza la revisión, debe informar al desarrollador y esperar hasta que le dé claridad antes de continuar. En Google, contratamos excelentes ingenieros de software, y usted es uno de ellos. Si no puede entender el código, es muy probable que otros desarrolladores no puedan hacerlo. De esta manera, también ayuda a los futuros desarrolladores a comprender este código al pedirle claridad al desarrollador.

Si el código es comprensible, pero no se siente lo suficientemente calificado para evaluar un determinado fragmento, asegúrese de que haya un revisor en el CL calificado, especialmente para problemas complejos como seguridad, concurrencia, accesibilidad, internacionalización, etc.

Contexto

A menudo es útil mirar CL en un contexto amplio. Por lo general, una herramienta de revisión de código solo muestra unas pocas líneas cerca del lugar de cambio. A veces es necesario mirar el archivo completo para asegurarse de que el cambio realmente tenga sentido. Por ejemplo, ve la adición de solo cuatro líneas, pero si observa el archivo completo, estas cuatro líneas están en el método de 50 líneas, que ahora realmente tiene que dividirse en otras más pequeñas.

También es útil pensar en CL en el contexto del sistema en su conjunto. ¿Mejora la calidad del código del sistema o lo hace más complejo, menos probado, etc.? No acepte confirmaciones que degraden el código del sistema. La mayoría de los sistemas son complicados por la suma de muchos pequeños cambios, por lo que es importante evitar incluso dificultades menores allí.

Bueno

Si ve algo bueno en la confirmación, avísele al desarrollador, especialmente cuando haya resuelto el problema descrito en uno de sus comentarios de la mejor manera posible. Las revisiones de código a menudo simplemente se centran en errores, pero también deberían alentar y valorar las buenas prácticas. Desde el punto de vista de la tutoría, a veces es aún más importante decirle al desarrollador qué hizo exactamente bien y no dónde cometió un error.

Resumen

Al ejecutar una revisión de código, asegúrese de que:

• El código está bien diseñado.
  
• La funcionalidad es buena para los usuarios de código.
  
• Cualquier cambio en la interfaz de usuario es razonable y se ve bien.
  
• Cualquier programación concurrente es segura.
  
• El código no es más complicado de lo que debería ser.
  
• El desarrollador no implementa lo que pueda ser necesario en el futuro con perspectivas desconocidas.
  
• El código está alineado con las pruebas unitarias apropiadas.
  
• Las pruebas están bien diseñadas.
  
• El desarrollador usó nombres claros en todas partes.
  
• Los comentarios son comprensibles y útiles, y básicamente responden a la pregunta ¿por qué? pero no que?
  
• El código está debidamente documentado (generalmente en g3doc).
  
• El código cumple con nuestras guías de estilo.

Durante la revisión, asegúrese de mirar cada línea de código, mirar el contexto , asegurarse de que está mejorando la calidad del código y felicitar a los desarrolladores por el bien que lograron hacer.

Navegación CL en revisión de código

Resumen

Ahora que sabe qué verificar en el código , ¿cuál es la forma más eficiente de realizar revisiones de código en varios archivos?

1. ¿Tiene sentido este cambio? ¿Tiene una buena descripción ?
   
2. Primero mira la parte más importante. ¿Está bien diseñado?
   
3. Mire el resto del CL en la secuencia apropiada.

Paso uno: echa un vistazo a todo el commit

Mire la descripción de CL y lo que hace en general. ¿Tiene algún sentido este cambio? Si no debería haber sido escrito inicialmente, responda inmediatamente con una explicación de por qué no es necesario. Cuando rechaza dicho cambio, también es bueno ofrecerle al desarrollador qué hacer en su lugar.

Por ejemplo, podría decir: "Parece que hiciste un buen trabajo, ¡gracias!" Sin embargo, en realidad vamos a eliminar el sistema FooWidget, por lo que no queremos realizar ningún cambio nuevo en este momento. ¿Qué tal si refactorizas nuestra nueva clase BarWidget?

Tenga en cuenta que el revisor no solo rechazó el CL actual y proporcionó una sugerencia alternativa, sino que también lo hizo cortésmente . Tal cortesía es importante porque queremos mostrar que nos respetamos como desarrolladores, incluso cuando no estamos de acuerdo entre nosotros.

Si obtiene bastantes CL no deseados, debe revisar el proceso de desarrollo en su equipo o las reglas publicadas para colaboradores externos para mejorar la comunicación al escribir el CL. Es mejor decirle a la gente "no" antes de que hagan un montón de trabajo que tendrá que desecharse o reescribirse en gran medida.

Paso dos: Aprenda las partes básicas de CL

Localice el archivo o archivos que representan la parte "principal" de este CL. A menudo hay un archivo con los cambios más lógicos, y esta es la parte principal de la CL. Primero mira estas partes principales. Esto ayuda a comprender el contexto de partes más pequeñas de la CL, y generalmente acelera la ejecución de la revisión de código. Si el CL es demasiado grande, pregúntele al desarrollador qué ver primero o pídale que divida el CL en partes .

Si ve problemas serios en la parte principal de la CL, debe enviar estos comentarios inmediatamente, incluso si no tiene tiempo para ver el resto de la CL en este momento. De hecho, mirar el resto del código puede ser una pérdida de tiempo, porque si los problemas son lo suficientemente significativos, muchas de las otras partes del código en cuestión desaparecerán y no tendrán importancia.

Hay dos razones principales por las que es tan importante enviar estos comentarios principales de inmediato:

• Los desarrolladores a menudo envían CL e inmediatamente comienzan un nuevo trabajo basado en él, mientras esperan el resultado de una revisión de código. Si el CL tiene problemas graves, tendrán que volver a trabajar el próximo CL. Es recomendable identificar los errores lo antes posible antes de que hagan mucho trabajo adicional además del diseño problemático.
  
• Los cambios importantes tardan más que las ediciones menores. Casi todos los desarrolladores tienen plazos. Para mantenerse dentro de ellos y no reducir la calidad del código, cualquier refactorización importante debe comenzar lo antes posible.

Paso tres: Desplácese por el resto del CL en la secuencia apropiada.

Después de asegurarse de que no haya problemas serios con el diseño del CL en su conjunto, intente descubrir la secuencia lógica para ver los archivos y asegúrese de que no se pierda nada. Por lo general, cuando observa los archivos principales, la forma más fácil es simplemente revisar el resto en el orden en que los presenta la herramienta de revisión de código. A veces también es útil leer primero las pruebas y luego el código principal, porque entonces tendrá una idea de cuál es el significado del cambio.

Velocidad de revisión de código

¿Por qué debería hacerse una revisión de código rápidamente?

En Google, optimizamos la velocidad de colaboración , no la velocidad de los desarrolladores individuales. La velocidad del trabajo individual es importante, simplemente no es tan prioritaria en comparación con la velocidad del trabajo en equipo.

Cuando observa lentamente el código, suceden varias cosas:

• La velocidad del equipo en su conjunto disminuye. Sí, si una persona no responde rápidamente a una revisión de código, hace otro trabajo. Sin embargo, para el resto del equipo, las nuevas funciones y las correcciones de errores se retrasan por días, semanas o meses, ya que cada CL espera una revisión de código y una revisión de código repetida.
  
• -. , , . , «» . (, ), , . - .
  
• La calidad del código puede sufrir. Disminuir la velocidad de la revisión del código aumenta el riesgo de que los desarrolladores no envíen CL tan buenos como podrían ser. Las revisiones lentas también dificultan la limpieza del código, la refactorización y las mejoras adicionales a los CL existentes.

¿Cómo ejecutar rápidamente una revisión de código?

Si no está en medio de una tarea enfocada, debe hacer un código de revisión poco después de que llegue.

Un día hábil es el tiempo máximo para una respuesta (es decir, esto es lo primero a la mañana siguiente).

Seguir estas pautas significa que un CL típico debe recibir varias rondas de revisión (si es necesario) dentro de un día.

Velocidad y distracción

Hay un momento en que la prioridad de la velocidad personal excede la velocidad del equipo. Si está en medio de una tarea enfocada, como escribir código, no se distraiga con la revisión del código. Los estudios han demostrado que un desarrollador puede tardar mucho tiempo en volver a un flujo de desarrollo sin problemas después de la distracción. Por lo tanto, la distracción de la codificación realmente le cuesta al equipo más que pedirle a otro desarrollador que espere un poco la revisión del código.

En cambio, espere un punto de interrupción en su trabajo. Por ejemplo, después de completar la tarea actual, después del almuerzo, regresar de una reunión, regresar de la cocina de la oficina, etc.

Respuestas rápidas

Cuando hablamos de la velocidad de revisión del código, nos interesa el tiempo de respuesta y no cuánto tiempo se requiere para todo el proceso hasta el final. Idealmente, todo el proceso también debería ser rápido, pero es aún más importante que las respuestas individuales lleguen rápidamente que todo el proceso .

Incluso si todo el proceso de revisión del código lleva mucho tiempo, la disponibilidad de respuestas rápidas del revisor durante todo el proceso simplifica enormemente la vida de los desarrolladores que pueden estar molestos por las respuestas "lentas".

Si está demasiado ocupado para una revisión completa inmediatamente después de recibir la solicitud, aún puede responder rápidamente con un mensaje sobre los plazos u ofrecer la revisión a otros revisores que pueden responder más rápido, oproporcione algunos comentarios generales iniciales (nota: nada de esto significa que debe interrumpir la codificación incluso para enviar dicha respuesta; envíe la respuesta a un punto de interrupción razonable en su trabajo).

Es importante que los revisores pasen suficiente tiempo revisando y asegurándose de que su "LGTM" significa "este código cumple con nuestros estándares ". Sin embargo, las respuestas individuales deberían idealmente ser rápidas de todos modos .

Revisión de código entre zonas horarias

Cuando trabaje entre diferentes zonas horarias, intente responder al autor mientras todavía está en la oficina. Si ya se fue a su casa, asegúrese de intentar enviar una respuesta antes de que regrese a la oficina al día siguiente.

LGTM reservado

Por razones de velocidad, hay ciertas situaciones en las que el revisor debe dar LGTM / aprobación, incluso en el caso de comentarios sin respuesta sobre el CL. Esto se hace si:

• El revisor confía en que el desarrollador considerará adecuadamente todos los comentarios restantes.
  
• Otros cambios son menores y opcionales .

El revisor debe indicar a cuál de estas opciones se refiere si esto no está claro.

Los LGTM reservados son especialmente útiles cuando el desarrollador y el revisor se encuentran en diferentes zonas horarias; de lo contrario, el desarrollador esperará todo el día para obtener la “Aprobación LGTM”.

CL grande

Si alguien le envía un código de revisión tan grande que no está seguro de cuándo puede verlo, entonces la respuesta típica sería pedirle al desarrollador que divida el CL en varios CL más pequeños . Esto suele ser posible y muy útil para los revisores, incluso si requiere un trabajo adicional del desarrollador.

Si CL ​​no puede desglosarse en CL más pequeños y no tiene tiempo para revisar rápidamente todo esto, al menos escriba algunos comentarios sobre el diseño general de CL y envíelos al desarrollador para su mejora. Uno de sus objetivos como revisor siempre debe ser "desbloquear" al desarrollador o permitirle tomar rápidamente cualquier otra acción sin sacrificar la calidad del código.

Mejoras de revisión de código a lo largo del tiempo

Si sigue estas recomendaciones y se acerca estrictamente a la revisión del código, encontrará que todo el proceso se acelera y se acelera con el tiempo. Los desarrolladores descubrirán qué se requiere para un código de alta calidad y le enviarán CL que son excelentes desde el principio, que requieren cada vez menos tiempo para verlos. Los revisores aprenden a responder rápidamente y no a agregar demoras innecesarias al proceso de revisión. Pero no comprometa los estándares de revisión de código o la calidad para una mejora imaginaria de la velocidad; de hecho, no logrará una aceleración general a largo plazo.

Situaciones de emergencia

También hay situaciones de emergencia cuando los CL tienen que pasar por todo el proceso de revisión del código muy rápidamente, y donde los principios de calidad tendrán que suavizarse. Pero lea la descripción de qué situaciones califican como emergencia y cuáles no.

Cómo escribir comentarios en la revisión de código

Resumen

• Se cortés.
  
• Explica tu razonamiento.
  
• Evite órdenes explícitas simplemente señalando problemas y dejando que el desarrollador decida.
  
• Aliente a los desarrolladores a simplificar el código o agregar comentarios en lugar de explicaciones simples para la complejidad.

Cortesía

En general, es importante ser cortés y respetuoso, y también muy claro y útil para el desarrollador cuyo código está viendo. Una forma de hacerlo es asegurarte de que siempre haces comentarios sobre el código y nunca sobre el desarrollador . No siempre tiene que seguir esta práctica, pero debe usarla cuando dice algo desagradable o controvertido. Por ejemplo:

Malo: "¿Por qué usaste transmisiones aquí si es obvio que la concurrencia no es beneficiosa?"

Bien: “El modelo de concurrencia aquí agrega complejidad al sistema, y ​​no veo ningún beneficio de rendimiento real. Como no hay una ventaja de rendimiento, lo mejor es que este código sea de un solo subproceso en lugar de usar múltiples subprocesos ”

Explica las razones

En el ejemplo "bueno" anterior, puede ver que ayuda al desarrollador a comprender por qué está haciendo su comentario. No siempre necesita incluir esta información en sus comentarios, pero a veces es apropiado dar un poco más de explicación sobre su lógica, las mejores prácticas que sigue o cómo su sugerencia mejora la calidad del código.

Direcciones

En general, hacer correcciones es tarea del desarrollador, no del revisor. No es necesario que desarrolle un borrador de solución detallado o que escriba código para el desarrollador.

Sin embargo, esto no significa que el revisor no pueda ayudar aquí. En general, se debe lograr un equilibrio apropiado entre identificar problemas y brindar asistencia directa. Señalar problemas y darle al desarrollador la oportunidad de tomar una decisión a menudo ayuda al desarrollador a aprender y facilita la revisión del código. También puede conducir a una mejor solución, porque el desarrollador está más cerca del código que el revisor.

Sin embargo, las instrucciones directas, sugerencias o incluso el código a veces son más útiles. El objetivo principal de una revisión de código es obtener el mejor CL posible. El objetivo secundario es mejorar las habilidades de los desarrolladores para que la revisión les lleve cada vez menos tiempo.

Aceptar la explicación

Si le pide al desarrollador que le explique un código incomprensible, por lo general, esto conducirá al hecho de que lo reescribirá más claramente . A veces, agregar un comentario también es una respuesta apropiada, si no es solo una explicación de un código demasiado complejo.

Las explicaciones escritas solo en la herramienta de revisión no ayudarán a los futuros lectores. Son aceptables solo en algunos casos, por ejemplo, cuando observa un área con la que no está muy familiarizado, y el desarrollador explica lo que los lectores de códigos comunes ya deberían saber.

Cómo superar la resistencia en el proceso de revisión del código

A veces, un desarrollador argumenta en un proceso de revisión de código. O no está de acuerdo con tu propuesta, o se queja de que eres demasiado estricto en general.

¿Quién tiene la razón?

Cuando el desarrollador no está de acuerdo con su propuesta, primero tómese el tiempo para considerar su posición. A menudo está más cerca de su código y, por lo tanto, puede tener una mejor idea de algunos aspectos. ¿Tiene sentido en su argumento? ¿Tiene sentido en términos de calidad de código? Si es así, entonces admite que tiene razón, y la pregunta desaparecerá.

Sin embargo, los desarrolladores no siempre tienen la razón. En este caso, el revisor debe explicar por qué cree que su propuesta es correcta. Una buena explicación demuestra tanto la comprensión de la respuesta del desarrollador como la información adicional sobre por qué se requiere el cambio.

En particular, cuando el revisor considera que su propuesta mejorará la calidad del código, debe insistir en ello si considera que la mejora de la calidad resultante justifica el trabajo adicional. La mejora de la calidad del código, como regla, ocurre en pequeños pasos.

A veces toma varias rondas de explicación antes de que sea realmente aceptado. Solo asegúrate de ser siempre cortés y hazle saber al desarrollador que lo escuchas, pero no estás de acuerdo.

Sobre el resentimiento de los desarrolladores

Los revisores a veces sienten que un desarrollador está molesto si un revisor insiste en mejorar. A veces los desarrolladores están realmente molestos, pero generalmente no por mucho tiempo, y luego estarán muy agradecidos de que los haya ayudado a mejorar la calidad de su código. Por lo general, si es cortés en sus comentarios, los desarrolladores no están realmente molestos en absoluto, y la preocupación solo está en la cabeza del revisor. Por lo general, el estilo de escribir comentarios es más frustrante que la persistencia del revisor con respecto a la calidad del código.

Ediciones pendientes

Una fuente típica de controversia es que los desarrolladores (por razones obvias) quieren hacer el trabajo. No quieren pasar por otra ronda de revisiones para este CL. Por lo tanto, dicen que eliminarán algo en el CL posterior, y ahora tiene que hacer LGTM para este CL. Algunos desarrolladores lo hacen muy bien e inmediatamente escriben otro CL que solucionará el problema. Sin embargo, la experiencia ha demostrado que cuanto más tiempo transcurra después del CL original, es menos probable que ocurra esta edición. De hecho, generalmente si el desarrollador no hace la edición de inmediatoeso usualmente nunca lo hace. No porque los desarrolladores sean irresponsables, sino porque tienen mucho trabajo y la edición se pierde u olvida en la rampa de otros trabajos. Por lo tanto, generalmente es mejor insistir en una solución inmediata antes de que la confirmación entre en la base de código y se "complete". Permitir ediciones pendientes es una forma típica de degenerar bases de código.

Si el CL introduce una nueva complejidad, debe ser reparado antes de enviarlo, si esto no es una emergencia. Si CL ​​muestra otros problemas que no se pueden resolver en este momento, el desarrollador debe registrar un error en el rastreador y asignárselo a sí mismo para que no se pierda. Opcionalmente, puede escribir un comentario TODO en el código con respecto a este error.

Quejas generales de gravedad

Si solía realizar una revisión de código bastante superficial y cambiar al modo estricto, algunos desarrolladores comenzarán a quejarse muy fuerte. El aumento de la velocidad de revisión de código generalmente resuelve estas quejas.

A veces pueden pasar meses hasta que desaparezcan las quejas, pero al final, los desarrolladores tienden a ver el valor de las revisiones estrictas de código, ya que ven lo bueno que es el código. A veces, los manifestantes más ruidosos incluso se convierten en sus partidarios más fuertes cuando sucede algo que les hace ver realmente el valor de las revisiones rigurosas.

Resolución de conflictos

Si está haciendo todo lo anterior, pero aún encuentra conflictos, consulte la sección Estándar de revisión de código para obtener recomendaciones y principios que pueden ayudar a resolver el conflicto.