Flujo de trabajo de revisión de código donde el autor de solicitud de extracción debe combinar

16

Varios equipos de mi empresa practican un flujo de trabajo de revisión de código que nunca antes había visto. Estoy tratando de entender el pensamiento detrás de esto, con la idea de que hay valor en hacer que toda la compañía sea consistente. (Contribuyo a múltiples bases de código y me he tropezado con las diferencias en el pasado).

  1. El autor del código presenta una solicitud de extracción
  2. El revisor examina el código
    • Si el revisor lo aprueba, deja un comentario en la línea de "Se ve bien, siéntase libre de fusionarse"
    • Si el revisor tiene dudas, deja un comentario como "Por favor, solucione los problemas menores X e Y, luego combínelos" (Para cambios importantes, regrese al paso 2)
  3. El autor del código realiza cambios si es necesario y luego combina su propia solicitud de extracción

Tengo las siguientes preocupaciones:

  • En el caso de la aprobación en el paso 3, este flujo de trabajo crea un viaje de ida y vuelta aparentemente innecesario para el autor de la solicitud de extracción. El revisor, que ya está mirando el código, podría fusionarlo de inmediato.

  • En el caso de que se soliciten cambios en el paso 3, la agencia para fusionar la solicitud de extracción ahora depende únicamente del autor del RP. Nadie además del autor mirará los cambios antes de la fusión.

¿Cuáles son algunas otras ventajas o desventajas de este flujo de trabajo? ¿Es este flujo de trabajo común en otros equipos de ingeniería?

aednichols
fuente
55
¿Le ha preguntado a la gente de su organización por qué eligieron este flujo de trabajo específico? Probablemente estén en una mejor posición para iluminar los méritos relevantes que nosotros. Simplemente estaríamos especulando.
Robert Harvey
1
¿Qué sucede en su organización cuando el revisor escribe "Por favor, solucione el problema principal X"?
Doc Brown
8
En mi experiencia, es mejor que el autor original sea el que haga la fusión en caso de que haya un conflicto de fusión por resolver. El autor original suele ser el que está mejor equipado para descubrir cómo resolver un conflicto de fusión.
17 de 26
Tendría curiosidad por la lógica aquí. Debería preguntar a sus colegas y escribirlo como una respuesta propia: aquí podría haber un muy buen proceso de pensamiento o justificación. Simplemente no puedo encontrar uno rápidamente.
Thomas Owens

Respuestas:

21

En el primer caso, generalmente es una cortesía. En la mayoría de las organizaciones, las fusiones inician una serie de pruebas automatizadas que deben abordarse rápidamente si fallan. Especialmente si hubo un retraso significativo entre el momento en que se envió una solicitud de extracción y cuando se revisó, es cortés permitir que se fusione en el calendario del autor, para que tengan tiempo de lidiar con cualquier consecuencia inesperada. La forma más fácil de hacerlo es dejar que se fusionen ellos mismos.

Además, a veces el autor se da cuenta de las razones posteriores de que una solicitud de extracción aún no debe fusionarse. Quizás las relaciones públicas de otro desarrollador tengan mayor prioridad y puedan causar conflictos. Tal vez pensó en un caso de uso descubierto. Tal vez un comentario de revisión provocó una lluvia de ideas que necesita más investigación antes de que el problema esté completamente satisfecho. El autor sabe más sobre el código, y tiene sentido darle la última palabra sobre cuándo se fusionó.

En el segundo punto, eso es solo una cuestión de confianza. Si no puede confiar en que las personas solucionen problemas menores sin que se verifiquen dos veces, no deberían estar trabajando para usted. Si el problema es lo suficientemente grande como para que necesite otra revisión después de la solución, entonces confíe en los revisores para pedir una.

Dicho esto, ocasionalmente fusiono las solicitudes de extracción de otros autores, pero generalmente son cambios muy simples o de fuentes externas, donde personalmente asumo la responsabilidad de guiar cualquier falla de automatización de prueba.

Karl Bielefeldt
fuente
2
Parece que hay más variedad de la que había imaginado. Mi experiencia con las pruebas automatizadas ha sido que las pruebas se ejecutan contra la rama antes de fusionarse, no después, convirtiéndose en una condición previa para la fusión. También he visto correcciones "menores" no revisadas, incluida la mía, que causan errores.
aednichols
2
Las pruebas a menudo se ejecutan como una condición posterior, así como una condición previa. Es completamente posible que los cambios de varias ramas entren en conflicto de maneras no obvias que no aparecerían como un conflicto de código y causarían que las pruebas comenzaran a fallar. En mi lugar de trabajo, requerimos que una rama esté actualizada con la rama base, así como todas las pruebas que pasan antes de que sea un candidato para la fusión y la fusión se produce automáticamente si se cumplen esas dos condiciones. No siempre tuvimos esa primera condición; antes de eso, en realidad teníamos problemas que se presentaban con poca frecuencia a pesar de que cada rama pasaba individualmente
Matthew Scharley
3

Hacer que el autor inicial combine su propia solicitud de extracción es mi flujo de trabajo preferido en equipos pequeños. Además de las ventajas técnicas ya mencionadas (en términos de resolución de conflictos de fusión, por ejemplo), creo que agrega valor a nivel cultural: crea un sentido de propiedad.

Especifiqué el autor inicial para el caso (raro) en el que otro desarrollador agregaría confirmaciones a la solicitud de extracción abierta (extrayendo la rama de la función de trabajo en progreso y volviendo a ella). Esto no sucede a menudo, y sería el resultado de una conversación en persona o por Slack: ¡Estos compromisos adicionales (por parte de otra persona) no deberían llegar allí por sorpresa! En este contexto, por autor inicial , me refiero al que envió la solicitud de extracción.

mkcor
fuente
2

En mi organización somos bastante nuevos en las solicitudes de extracción y su pregunta es una que me he reflexionado.

Una observación que me gustaría agregar: en algunas herramientas (utilizamos TFS) podría haber un elemento de trabajo asociado con la solicitud de extracción.

Si ese es el caso, se vuelve un poco complicado tener en cuenta cuándo el revisor realizó la fusión. En ese escenario, el desarrollador debe volver a visitar el RP, abrir el error o solicitar la modificación y marcarlo como 'resuelto'. Si lo marcamos como "resuelto" demasiado pronto, los evaluadores creen que la solución ya forma parte de la compilación actual.

TFS 2017 mejoró su implementación de la solicitud de extracción. Ahora el desarrollador puede solicitar la solicitud de extracción para fusionarse automáticamente si se cumplen todas las condiciones (sin conflicto de fusión, aprobación de los revisores y sin compilaciones rotas). YMMV.

9Runa5
fuente
1

De esta manera, el autor tiene la oportunidad de cambiar de opinión acerca de su rama, tal vez descubrió algo que debería hacerse de una manera diferente y volver a poner los cargos adicionales para su revisión.

gnasher729
fuente