¿Cuál es la mejor manera de revisar un código antes de que se comprometa con el tronco? (SVN)

¿Cuál es la mejor manera de revisar un código antes de que se comprometa con la troncal SVN? Una idea en la que estoy pensando es hacer que el desarrollador confirme su código en una rama y luego revise su código mientras combina las revisiones de la rama en el tronco. ¿Es esta una buena practica? Si no, ¿qué más se podría hacer para revisar un código antes de que se confirme en el enlace troncal?

Sin embargo, hay dos escuelas: lo que usted propone o "revisa antes de comprometerse". La mayoría de las diferencias se pueden ver como negativas y / o positivas. - por ejemplo, sin seguimiento de los cambios causados ​​por una revisión: ¿desea verlos como confirmaciones discretas o solo le interesa el trabajo final?

Revise antes de confirmar: no se requiere ramificación (aunque se puede hacer si lo desea), debe dar acceso a los revisores a las carpetas de trabajo. El código se puede cambiar durante y después de la revisión sin seguimiento. Las correcciones causadas por la revisión no aparecen en el repositorio.

Revisar después de confirmar (en una rama): es necesario girar una rama para cada revisión (aunque esto puede estar en el flujo de trabajo). El código enviado para su revisión no se puede cambiar sin el seguimiento de los cambios. Alguien tiene que fusionar las ramas revisadas y realizar un seguimiento de lo que se ha revisado y lo que no se ha revisado.

Depende en gran medida de la cultura y experiencia del equipo. ¿En qué confía el modelo, cuál es el objetivo principal de las revisiones? Personalmente prefiero la revisión después de la confirmación, ya que permite realizar un seguimiento de los cambios como resultado de la revisión. Ahora usamos Git y Gerrit, ya que proporcionan un buen equilibrio entre las dos opciones.

1. Antes de comprometerse, ejecute 'svn diff' para generar un archivo de parche.
2. Envíe el archivo de parche al revisor que lo aplica a una copia limpia del tronco.
3. El revisor repasa los cambios utilizando el visor de diferencias elegido.
4. El revisor realiza la compilación y ejecuta pruebas.
5. Si todo va bien, dígale al desarrollador que pueden verificar sus cambios. Si
   hay problemas, el desarrollador vuelve al paso 1.

Está el mundo ideal, y luego está el mundo real.

En el mundo ideal , todo su código se prueba, por lo que puede estar seguro de que todo lo que se registre funcionará o sabrá que está roto porque falla una o más pruebas. Además, cualquier persona que no tenga tanta experiencia se combinará con alguien que tenga experiencia, por lo que la revisión del código se realiza sobre la marcha (usted se compromete a medida que avanza).

En el mundo real , las cosas son diferentes. Las empresas quieren ese cambio en vivo ahoray le dirá, con una cara perfectamente seria, que sí, tendrá tiempo para limpiar el código y agregar casos de prueba más tarde. Probablemente no tenga tiempo para revisar todo el código y el porcentaje de código cubierto por las pruebas disminuye continuamente. La razón principal para la revisión del código es que los desarrolladores junior aprendan de los desarrolladores senior (cuando haya tiempo para eso) haciendo que una persona más experimentada revise los cambios y sugiera "mejores formas de hacer las cosas (TM)". Tendrás desarrolladores senior que solo cometerán código no revisado. La ramificación solo para una revisión de código y luego la fusión es una gran pérdida de tiempo. Una forma de superar este problema es declarar una reunión de equipo semanal regular de 2 horas (más o menos) en la que elija uno o dos cambios recientes en los que las personas trabajaron con poca antelación y haga que sus autores "presenten" su enfoque al mirar el código juntos en un proyector o algo así Esto puede llevar a algunas discusiones interesantes (generalmente se sale bastante del tema) pero generalmente mejora la comprensión de todos sobre cómo hacerlo correctamente. Además, la presión de posiblemente tener que presentar su código hace que algunas personas lo hagan mejor ;-)

O puede ser afortunado y trabajar en un entorno del mundo real donde no es tan agitado, los programadores son realmente apreciados por lo que hacen en lugar de maltratados, y hay tiempo para hacer todo bien. En cuyo caso, mi respuesta sería: pruebe algunos de los diferentes métodos sugeridos en las respuestas aquí y vea cuál se adapta a su equipo y la forma en que trabaja mejor.

Las sucursales deberían funcionar bien, según mi experiencia de usarlas en revisiones previas a los compromisos en trabajos anteriores.

_{Tenga en cuenta que en ese entonces, estábamos usando revisiones previas a la confirmación solo para parches críticos para el código de candidato de lanzamiento de producción, por lo que no había muchas ramas (los cambios de rutina se pasaron a través de revisiones posteriores a la confirmación).}

Como parece que va a utilizar revisiones previas a la confirmación para todos los cambios, es posible que deba administrar una gran cantidad de sucursales. Si espera que el desarrollador haga un cambio "revisable" por semana en promedio, terminará teniendo aproximadamente 50 sucursales cada año por cada desarrollador en equipo. Si está utilizando porciones de trabajo más pequeñas, como las que toman 1, 2, 3 ... días, multiplique 50 por 2, 3, 5 ... en consecuencia.

A continuación hay algunas otras consideraciones a tener en cuenta si lo desea de la mejor manera .

1. manejo de casos cuando la revisión retrasada bloquea el código necesario para otros miembros del equipo

Establezca, monitoree y resuelva conflictos relacionados con los plazos de revisión de código. Según mi recuerdo de las revisiones previas a la confirmación de los cambios de rutina que traté en uno de los proyectos anteriores, el plazo razonable es de aproximadamente 3 días y el tiempo para comenzar a preocuparme es cuando la revisión no se completa más de 1 día después del envío.

_{A modo de comparación, en las revisiones posteriores al compromiso, estos requisitos son mucho más relajados (estoy usando el plazo de 2 semanas y empiezo a preocuparme después de 1 semana), pero dado que apunta a las revisiones previas al compromiso, esto probablemente no sea interesante.}

2. fusionar conflictos al enviar el código revisado

¿Qué hacer si el compromiso para el código revisado está bloqueado por cambios conflictivos cometidos por otra persona mientras el código estaba esperando su revisión?

Un par de opciones a considerar son

• retroceda al inicio y exija a los desarrolladores que vuelvan a implementar y repasen el cambio.
  Para ese caso, es posible que deba abordar un impacto negativo en la moral del equipo que esto podría (¡lo hará!).
• pasar la responsabilidad de la fusión a otro miembro del equipo ("maestro de fusión")
  Para ese caso, también deberá decidir si las fusiones en sí deben pasar por una revisión previa al compromiso o no, y en caso afirmativo, qué hacer en caso de que si esa fusión a su vez se encuentra con otro conflicto.
• ignorar los cambios realizados en el código revisado en la etapa de fusión
  Para ese caso, es posible que deba abordar un impacto negativo en la moral del equipo relacionado con el hecho de que el código confirmado difiere del que se revisó.
• invente una forma de evitar conflictos El
  enfoque directo es permitir que solo un desarrollador a la vez modifique un conjunto particular de archivos, aunque esto no lo protegerá del tipo de cambios que no modifican los archivos directamente, sino que los impacta a través de cambios internos de API . También puede descubrir que este tipo de "bloqueo pesimista" hace que los cambios en todo el sistema y la refactorización profunda sean bastante problemáticos.

_{A modo de comparación, no habría problemas de ese tipo en las revisiones posteriores a la confirmación (ya que se trata de código que ya está fusionado por definición), pero dado que apunta a las revisiones previas a la confirmación, probablemente esto no sea interesante.}

3. Desarrollador de carga que está esperando revisión

Establezca una política explícita para determinar si el desarrollador que envió la revisión debe cambiar a una nueva tarea o hacer otra cosa (por ejemplo, perseguir al revisor).

_{A modo de comparación, las revisiones posteriores a la confirmación apenas necesitan una política explícita (ya que es natural proceder a la siguiente tarea después de haber confirmado el código y teniendo en cuenta que el plazo de revisión es una o dos semanas), pero dado que se dirige a las revisiones previas a la confirmación, esto probablemente sea no es interesante.}

Cualquier pieza de desarrollo que necesite una revisión tendría que estar en una rama separada. Por lo tanto, la rama ya debería existir antes de que llegue el momento de la revisión. Entonces el paso debería ser:

1. revisión
2. Revisar (posiblemente)
3. volver a la revisión (posiblemente)
4. Combinar en el maletero

La fusión es lo difícil. Cuanto más tiempo permanezca independiente la rama, más difícil será fusionarla nuevamente en el tronco. (También podría ser más difícil de probar).

La fusión cruzada es una posible solución. Antes de fusionarse con el tronco (paso 4, o incluso antes, digamos antes del paso 3 o el paso 1), combine el tronco con la rama. El desarrollador puede hacerlo y probarlo. Luego, la rama alcanza el tronco y se hace más fácil fusionarlo con el tronco. La mejor manera de fusionarse con el baúl es usted o quien esté a cargo del baúl.

Algunas personas intentan rebase en lugar de fusión cruzada. Algunas personas argumentan que rebase es malo. Ese es otro debate.