Mi empresa recientemente comenzó a hacer revisiones de código formalizadas. El proceso es el siguiente: se envía a un github, solicita una solicitud de extracción, el código es revisado por aproximadamente tres personas, luego, si todo pasa, su código ingresa.
El proceso parece justo, sin embargo, las tres personas que hacen las revisiones del código no parecen ser justas. Noté que cuando pongo mi código para su revisión, obtengo entre 100 y 200 comentarios. El número más alto para mí fue 300 comentarios una vez. Por supuesto, pensaría que se trata de grandes cambios, pero también pueden ser cambios muy pequeños con menos de 50 líneas de código (que incluye pruebas unitarias). Todos los comentarios se consideran "imprescindibles" y sin argumento.
Con eso en mente, mi principal problema aquí es que parece un poco excesivo. Hablé con el grupo y me dijeron básicamente que solo porque tuve tantos años de desarrollo en php no significa que soy un "desarrollador". Por supuesto, esto parece más doloroso que no. También noto que dentro del grupo, no parecen producir tantos comentarios y la mayoría de las veces ignoran o ignoran otros comentarios o sugerencias que rara vez lo aceptan como un punto válido, incluso si algo está roto.
¿Entonces mi pregunta es si esto es justo? O común?
fuente
Respuestas:
En mi humilde opinión, ese es el problema real, ya que no hay priorización en eso. Cuando recibe 100-300 comentarios, debe haber algunos de ellos que tengan prioridad A (errores reales), algunos de ellos prio B (que probablemente conduzcan a errores más tarde) y algunos de ellos prio C (todo lo demás). Dígale a sus colegas que está dispuesto a respetar todos sus deseos, pero para que los cambios sean efectivos y su tiempo es limitado, insista en una priorización. Luego, comience arreglando el comentario del prio A primero, y si realmente tiene tiempo para más después de eso, puede comenzar con B (si tiene suerte, su jefe entenderá que arreglar el prio B y C no es tan importante, y le dará algunas tareas más importantes en lugar de perder el tiempo).
fuente
Las revisiones de código pueden ser un proceso divisivo.
Sin embargo, estás en un cruce importante. Haga un análisis reflexivo sobre su revisión. ¿Están identificando problemas de selección de liendres o resaltando fallas serias en su estilo y lógica?
Si es lo primero, recomendaría trabajar hacia una resolución (nuevo trabajo o nuevos procesos de revisión de código).
Si es lo último, le recomiendo leer mucho el código y estudiar para intentar que su código sea de calidad profesional.
fuente
Por sus comentarios, parece que sus colegas están utilizando el proceso de revisión del código para acordar una metodología o pulir el código. Acabo de comenzar a hacer revisiones de código como usted y me doy cuenta de que a veces discutimos mucho sobre cosas que son solo enfoques de implementación o mejoras. Esto no está nada mal en la medida en que sea razonable (300 comentarios me parecen demasiados, eso debe parecer un hilo de reddit)
Tal vez necesite acordar algunas decisiones arquitectónicas sobre el código antes de comenzar a implementarlo o tal vez simplemente esté de acuerdo sobre las convenciones de nombres, patrones y buenas prácticas para que todos sepan lo que se considera "buen código".
Si cumple con los estándares de su código como dice y el código funciona según lo previsto, no debería haber tantos comentarios, por lo que están utilizando su código como foro o lo están controlando, ya que parece que está señalando.
Intentaría ser crítico conmigo mismo, trataría de participar en las conversaciones y ver la razón de todos estos comentarios y tal vez hablar con ellos al respecto de una manera constructiva para ver por qué están tan descontentos con su código y si puede código de una manera que hace felices a todos y el trabajo no se atasca en la revisión de código.
Acabo de leer sus últimos comentarios, a veces, cuando no está de acuerdo con el código, puede revisarlo cientos de veces y proponer cambios en todas partes que no lo hagan feliz porque la verdadera razón es que habría tomado una decisión arquitectónica diferente. y simplemente no le gusta ese código, no importa cuántas veces lo refactorice. Como dije anteriormente, tal vez necesite acordar el enfoque del código de antemano para que cuando lo escriba sepa lo que esperan de él y, por lo tanto, su código sería más razonable para ellos.
fuente
Por lo que dices, me parece que podrían tener un sesgo en contra de los desarrolladores de php, y por lo tanto están tratando de encontrar cada cosa que esté mal con tu código para probar su punto.
Con respecto a la revisión del código en sí, creo, como ya dijiste, que una cantidad tan grande de comentarios menores es menos útil que algunas críticas buenas y válidas. Y aunque tengo una experiencia limitada con respecto a las revisiones de código, la siguiente técnica ha funcionado bien para los equipos en los que trabajé en el pasado.
Además, tengo que decir que mis primeras revisiones de código real también contenían más comentarios de los que originalmente esperaba. Sin embargo, nunca consideré esto como algo malo. Si continúa aprendiendo de sus comentarios² y está dispuesto a aplicar esas técnicas / mejores prácticas recién aprendidas en sus futuras presentaciones de código, los comentarios deberían ser menos. Seguramente fue el caso para mí ;-)
¹ En mi experiencia, esto sucede mucho, ya que muchos programadores afirman que php es el lenguaje de programación más malvado, ya que los programadores más inexpertos lo usan. ¡Me distancio de esta afirmación porque creo que un excelente software se puede escribir en cualquier idioma!
² Suponiendo que aunque los comentarios son excesivos, hay algo de valor en ellos
fuente
¿Es común que alguien obtenga más de 100 comentarios en sus revisiones de código de forma rutinaria? Yo diría que no. Es común que las personas cuya calidad de código "deja mucho que desear" reciban muchos comentarios, absolutamente.
Sin embargo, también depende de las "reglas" del proceso de revisión del código. TODOS tienen sus propias ideas sobre cómo se debería haber hecho algo. Si su proceso de revisión de código permite que los comentarios tengan la forma "Debería hacerlo de esta manera en lugar de hacerlo de esa manera", entonces es probable que reciba MUCHOS comentarios incluso para el código adecuado. Si su proceso está destinado a encontrar "defectos", entonces el número de comentarios debería ser mucho menor.
En mi experiencia, las revisiones que permiten "sugerencias" de métodos alternativos son una pérdida de tiempo. Esas "sugerencias" deben manejarse una a una fuera del proceso de revisión. Las revisiones de defectos son más útiles ya que hacen que las personas se centren en los errores en lugar de "¿por qué no lo hiciste como yo lo hubiera hecho?". También es más útil porque no se puede negar un error si alguien encuentra uno. Por lo tanto, no hay sentimientos heridos, sino una probable gratitud en su lugar.
ACTUALIZACIÓN: Con todo lo dicho, algunos códigos son simplemente malos, incluso si están libres de defectos. En ese caso, el comentario de revisión debe ser un solo comentario que diga algo así. "Este código necesita ser limpiado. Por favor posponga la revisión hasta que el código sea discutido con [su nombre aquí]". En ese caso, la revisión adicional del código debería detenerse hasta que se rectifique el comentario.
ACTUALIZACIÓN2: @Usuario: ¿Discute su código / diseño con uno de ellos mientras lo desarrolla para que pueda implementar lo que está buscando antes de llegar lejos a su manera? ¿Está cambiando algo acerca de cómo está desarrollando el código en función de sus sugerencias o sigue pensando que su camino está bien? ¿Estás aprendiendo algo de sus comentarios?
Cuando soy el líder de un proyecto, mi trabajo es ser responsable de TODOS los productos de trabajo. Si apruebo un producto de trabajo, estoy afirmando que el producto es aceptable. Quiero tener una reputación de construir productos de calidad. Por lo tanto, tengo expectativas y no aceptaré menos que satisfactorio. Al mismo tiempo, trato de enseñar y explicar las razones de mis preferencias. Es posible que esas preferencias no siempre sean ideales (particularmente a los ojos de los demás), pero la mayoría de esas preferencias provienen de la experiencia. Suele ser una reacción para evitar repetir las malas. Por lo tanto, hay algunos de mis "fanáticos" personales que son necesarios para obtener mi aprobación, independientemente del rechazo.
Por otro lado, debe conocer las expectativas necesarias para obtener la aprobación de sus productos de trabajo. Puede estar en desacuerdo, pero dado que no parece tener la autoridad para descartar, aprenda lo que se espera. Dudo que el equipo esté tratando de hacerte fracasar. Como eso los hace quedar mal también. En ese sentido, solo demuestre que está ansioso por aprender (incluso si no lo está), tome lo que dicen y haga todo lo posible para adaptarse a sus preferencias y es probable que los vea retroceder un poco. Tal vez encuentre el que al menos pueda tolerar y vea si van a tomar un poco de la mano para enseñarle sus formas. Quién sabe, en el proceso puedes aprender algo que realmente podría llevar tus habilidades al siguiente nivel.
fuente
Algunas diferencias importantes con el proceso de inspección de nuestro equipo:
fuente
Para 50 comentarios LOC 300 parece ser un poco excesivo y - wow - 3 revisores por cada solicitud de extracción? Su empresa debe tener muchos recursos.
Según mi experiencia para un proceso útil de revisión de código, debe haber algunas reglas y / o pautas:
Si los revisores no le dan prioridad, pregunte a su gerente de proyecto / líder de equipo responsable; La persona responsable de los costos debe tener una opinión sobre las prioridades.
Si tiene una arquitectura definida, una comprensión común de los patrones de diseño que utiliza en su proyecto y un estilo de código acordado, entonces los comentarios de revisión deben ser solo sobre "problemas reales" como problemas de seguridad, errores no intencionales, casos de esquina no cubiertos por el especificado arquitectura, etc.
Si su equipo de desarrollo no ha acordado los "problemas de sabor" (p. Ej., Si una variable miembro comienza con "m_"), cada revisor lo obligará a seguir su estilo, lo cual es una pérdida de tiempo / dinero.
fuente
Esto realmente me suena como un problema de comunicación. Tiene la expectativa de que su código no sea lo suficientemente malo como para merecer 300 comentarios. Los revisores parecen pensar que necesita muchos comentarios. Discutir de un lado a otro de forma asincrónica va a perder mucho tiempo. Diablos, escribir 300 comentarios es una pérdida de tiempo tremenda. Si no se trata de todos los defectos, como nuevo miembro del equipo es posible que aún no conozca el estilo del equipo. Eso es normal y algo que se debe aprender para acelerar todo el equipo.
Mi sugerencia es ahorrar tiempo. Acelera la retroalimentación. Me gustaría:
La gente puede argumentar en contra del emparejamiento porque "tomará más tiempo", pero eso obviamente no es un problema aquí.
fuente