¿Qué es una mentalidad útil cuando se realiza una revisión formal del código?

14

Nuestro equipo recientemente comenzó a realizar revisiones de código para cada registro.

Como líder del equipo, estoy tratando de encontrar un equilibrio entre proporcionar demasiadas sugerencias, molestar a los desarrolladores y disminuir la producción del equipo, y dejar ir el código que habría escrito de manera diferente.

¿Existe alguna evidencia, estudio u orientación de fuentes bien conocidas que sugiera un enfoque útil?

Kye
fuente
2
La primera pregunta que debe hacerse : ¿por qué está haciendo revisiones de código?
Philip Kendall
1
Me sentiría tentado a asignar algún tipo de "importancia" a cada comentario. Vulnerabilidad crítica de seguridad = muy alta importancia. Error = importancia normal. Formateo de código = importancia cero (culpe a las herramientas que no se reformatean automáticamente de la forma que desee y no al programador).
Brendan
Podría interesar a usted
Laiv
La forma en que una persona se acerca o responde a una revisión de código dice mucho sobre su capacidad para mantener la objetividad y pensar críticamente. A veces los desarrolladores necesitan capacitación para este propósito específicamente.
Frank Hileman

Respuestas:

15

Tenga en cuenta los objetivos generales: al final, solo el software de trabajo importa

La revisión por pares y la revisión del código de registro tienen el objetivo de mejorar la calidad . Pero no hay nada peor para la calidad que un desarrollador desmotivado. Como líder del equipo, su función no es respaldar el código como algo que podría haber escrito usted mismo, sino promover el trabajo en equipo y garantizar el resultado general.

Establezca un alcance claro para su revisión

Tenga en cuenta: no es su código, sino el código del equipo. Por lo tanto, concéntrese en cosas que podrían conducir a resultados incorrectos.

  • No cuestione la forma en que su desarrollador ha elegido cumplir los requisitos, a menos que esté seguro de que no funcionará (pero ya debería haber fallado las pruebas, ¿no?)

  • No desafíe el bajo rendimiento a menos que haya una medida que muestre dónde está el problema. La optimización temprana es la raíz de todo mal ;-)

  • Si encuentra un diseño o una estructura de software para desafiar, ¡pregúntese por qué no se descubrió por adelantado! El código ya escrito es costoso de reescribir. Si esto sucede, es hora de revisar el desarrollo de software y las prácticas de trabajo en equipo al menos tanto como el código.

  • abordar el cumplimiento de los estándares de codificación establecidos. Es el tema más molesto para discutir tanto para el revisor como para el revisado. Cuando todos acordaron usar nombres de clase en mayúscula en su equipo y un chico tiene una clase sin él, ¿es cuestión de gustos? ¿O de la efectividad y el riesgo del trabajo en equipo?

Por cierto, si cree que no vale la pena discutir un estándar de codificación, elimínelo de sus estándares y no pierda tiempo y energía.

Desarrollar liderazgo: el lado humano de la revisión

Como líder de equipo, puede encontrar aquí una oportunidad para desarrollarse y desarrollar su equipo, más allá de la formalidad de un control de calidad:

  • Las revisiones de código son mucho más agradables para todos, si hay un verdadero intercambio. Déle a su desarrollador la oportunidad de mostrar sus habilidades (y sí, tal vez podría aprender algo nuevo).
  • Esté atento a las críticas sobre las opciones de diseño o los estándares existentes. A veces las personas pueden hacer frente mejor a tales frustraciones, solo porque pueden hablar de ello.
  • Entrene a sus juniors: no dude en dar consejos o refactorizar las orientaciones para la próxima iteración. No hagas esto con personas mayores: en otro mundo, tu respectivo papel podría haber cambiado.

Aprovecha otras prácticas

Hay un par de cosas que puede evitar en la revisión de código:

  • El uso de un analizador de código estático en su cadena de compilación puede automatizar la búsqueda de errores comunes , o construcciones no portátiles, mucho antes de la revisión por pares. Algunas herramientas incluso pueden verificar algunos de sus estándares de codificación .
  • Si tiene estándares sobre el diseño del código, use un formateador previo de impresión bonita o formateadores similares para formatear el código según se requiera automáticamente. Nunca pierdas el tiempo en algo que un software pueda hacer por ti mejor y sin discutir :-)
  • Finalmente, la calidad no solo se garantiza mediante la revisión, sino también mediante pruebas. Si aún no usa TDD , piense independientemente de la revisión del código.
Christophe
fuente
"Software de trabajo"? No muy útil "Software correcto": ¡eso es lo que prefiero!
Frank Hileman
@FrankHileman De hecho! Y precisa, confiable, utilizable, eficiente y adecuada para su propósito. Es solo una cuestión de definir el término "trabajando" apropiadamente ;-)
Christophe
3

Como desarrolladores somos, la mentalidad debe permanecer siempre abierta y escéptica al mismo tiempo.

Abierto, porque no sabemos cuándo un desarrollador puede sorprendernos, y escéptico sobre nuestras propias ideas porque a menudo olvidamos que en la ingeniería de software no hay una única forma correcta de implementar una solución. La lógica detrás de nuestras soluciones podría tener sentido para nosotros y no tener ninguna para los demás. Detrás de un olor a código podría haber una gran idea. Tal vez, el desarrollador no encontró la manera de expresarlo correctamente.

Debido a que nosotros (los humanos) somos terribles en la comunicación, no haga suposiciones falsas, esté dispuesto a preguntar al propietario del código sobre el código que está revisando. Si él / ella no pudo codificar la idea bajo los estándares de la compañía, como desarrollador principal, esté dispuesto a guiarlo también.

Aquí el enfoque subjetivo. El enfoque objetivo, la OMI, se explica muy bien en esta pregunta .

Además del enlace anterior, el conjunto de objetivos a alcanzar (mantenibilidad, legibilidad, portabilidad, alta cohesión, acoplamiento flojo, etc.) no son necesariamente los Diez Mandamientos. Usted (el equipo) debería poder adaptar estos objetivos a un punto en el que el equilibrio entre calidad y productividad haga que el trabajo sea cómodo y "habitable para los desarrolladores".

Sugeriría el uso de herramientas de análisis de código estático para medir el progreso de la calidad de acuerdo con estos objetivos. Herramientas como SonarQube nos proporcionan Puertas de calidad y Perfiles de calidad que se pueden personalizar de acuerdo con nuestras prioridades. También nos proporciona un rastreador de problemas, donde los desarrolladores pueden ser identificados con problemas relacionados con el olor del código, errores, prácticas dudosas, etc.

Este tipo de herramientas puede ser un buen punto de partida, pero como dije, manténgase escéptico. Puede encontrar que algunas reglas en Sonar no tienen sentido para usted, así que no dude en ignorarlas o eliminarlas de su perfil de calidad.

Laiv
fuente
2

Entrometerse con el código del desarrollador para los cambios cosméticos desmotivará al desarrollador, pero en circunstancias absolutas debe hacerse. El líder tiene que encontrar el equilibrio entre proporcionar una revisión útil del código y aprender a dejar de lado las deficiencias menores. https://blog.smartbear.com/sqc/for-the-new-team-lead-the-first-six-things-you-should-know/

Nishanth Menon
fuente
¿Cuáles son las "circunstancias absolutas" que requieren cambios cosméticos?
Ewan
1
Cuando no se han seguido las pautas de codificación, los defectos de diseño del código que pueden causar pérdidas de memoria son casos en los que el líder no puede permitirse el lujo de dejarlo ir.
Nishanth Menon
Su enlace está muerto
Greenonline
1

Algunas cosas para tener en mente:

  1. Se trata tanto de la psicología como de la tecnología, por lo que no hay una regla de oro aquí.
  2. Lo que se trata de las personas no se trata solo del conocimiento, sino también de la cultura y la posición en la jerarquía.
  3. Si se trata de un juego "largo" (empresa estable y establecida), un equipo bien integrado en el que las personas confían unas en otras generalmente tiene un valor más alto que el código que se está revisando. No debe usarse para forzar cosas que no son absolutamente necesarias para proceder. El diablo está en los detalles ...
  4. Si se trata de un juego "corto" (proyecto paralelo, I + D, muchos freelancers en un grupo), los costos de CR a menudo superan las ventajas de hacerlo. Y la actitud debería ser "solo hazlo funcionar"
smentek
fuente
-4

Solo hay dos cosas que importan.

  1. ¿Existen pruebas automatizadas para todos los requisitos?
  2. ¿Todos pasan?

Todo lo demás es cosmético y debería discutirse sobre la cerveza en lugar de imponerse como una puerta.

Si sigue esta vista, lo limita a un área de enfoque estrecha.

¿Son buenos los requisitos? Lo que idealmente debe saber antes de comenzar la tarea, es decir, el rendimiento, la seguridad, etc., deberían estar allí

¿Las pruebas son buenas pruebas? ¿Hay casos límite omitidos? ¿Están probando bien el requisito, etc. Finalmente: ¿Puede escribir una prueba que sea para un requisito existente pero que fallará?

Ewan
fuente
33
¿Diría que es aceptable un código sin saltos de línea, con solo nombres de variables de una sola letra y ofuscado? Todas las pruebas pasarían, pero no es mantenible .
Philip Kendall
En una revisión de código? si. Tan pronto como empieces a nombrar, todo es subjetivo. Solo tiene que elegir un ejemplo extremo, pero hay un montón de casos en que las personas utilizan iteradores de una sola letra o código condensan en funciones de una sola línea que se considera una buena práctica
Ewan