Cómo hacer que los desarrolladores realicen revisiones de código de manera oportuna

12

La compañía para la que trabajo requiere que otros desarrolladores revisen todo el código antes de que se comprometa. Los miembros de mi equipo a menudo se sienten frustrados porque los otros desarrolladores están demasiado ocupados codificando para hacer una revisión, especialmente si es muy largo. ¿Cómo incentivas a otros desarrolladores para que realicen revisiones oportunas del código?

(Utilizamos git-svn para poder continuar codificando mientras esperamos una revisión. Sin embargo, todavía me resulta frustrante cuando tengo que esperar mucho tiempo antes de poder confirmar mi código).

code-reviews titaniodecoy
fuente

Respuestas:

12

Mira cómo Facebook lo hace con su propia aplicación, llamada phabricator: http://phabricator.org/

Básicamente se comprometen por tema y, para cada problema, se muestra el código, que debe ser revisado por alguien. El código no entra en su repositorio principal hasta que el revisor dice que está bien hacerlo.

Supongo que lo hace más divertido.

Además, quizás se deba asignar un código a dos personas: una que lo haga y otra que lo revise.

Aunque quizás tus compañeros de equipo no crean en esta crítica.

Personalmente, a falta de revisores, utilicé pruebas unitarias para funciones de nivel inferior y "la prueba de conserje" para todo lo demás: la prueba de conserje se llama de esa manera, porque incluso el conserje debería poder entender su código.

Por lo general, eliminé algunas partes menores, como corchetes de alcance de bloque / función, anotaciones de visibilidad, a veces incluso tipos, y se lo mostré a gerentes, expertos en dominios, compañeros, quien solicitó el código: "¿es esto lo que quieres?"

Además, ir allí personalmente y no irse hasta que se complete la revisión ayuda.

O, en caso de que no estés bien con el equipo, o ellos no estén bien contigo, ya sabes, "si puedes cambiar la compañía, cambia de compañía" ...

Aadaam
fuente
9

Voy a basar esto en un par de suposiciones:

  1. Todo el mundo parece querer escribir código (si no, tienes personas que necesitan ir).
  2. Todos quieren que se registre su propio código.

Solo permita que quienes completen sus revisiones tengan su código registrado.

Tal vez se pueda dedicar un cierto bloque de tiempo a las revisiones de código con la esperanza de evitar el problema de ser interrumpido.

El objetivo debe ser verificar el código de calidad. No desea castigar / forzar las revisiones hasta el punto en que todos simplemente le den una aprobación de "sello de goma".

Si tiene diferentes niveles (jr., Sr., Etc.), la promoción y el mantenimiento de un título deben depender de hacer su trabajo.

JeffO
fuente
1

En un par de empleadores anteriores, rotamos quién estaba en Revisión de Código diariamente. Por lo general, 3 personas compartieron un día de RC y dos de los revisores tuvieron que firmar cada CR. Esto aseguró que cuando era tu día, supieras que CR se esperaba de ti, independientemente de tus otros proyectos. Así que cada cinco días más o menos, era tu turno y podías ajustar tus tareas de desarrollo en consecuencia.

Actualmente, tenemos un jefe de equipo que realiza un CR superficial en el código de su equipo. Dependiendo de qué área de la aplicación se haya actualizado, una vez que se complete el CR, se puede enviar al Equipo de Revisión Global, donde verificamos las cosas que tienen un impacto global en las aplicaciones, en lugar de las cosas que son relacionado con una sola característica. Cuando hay que hacer una gran revisión, generalmente la dividimos entre unas pocas personas para que nadie tenga que lidiar con los cambios en una cantidad ridícula de archivos.

Dicho esto, solo estamos revisando el código que se ha comprometido con la rama / variante Dev actual. El código tiene que pasar Revisión de Código, Revisión Global, Revisión de DB y Revisión de UI (cada una según sea necesario) antes de que pueda promoverse al siguiente entorno (por ejemplo, Alfa).

Finalmente, aceptamos un acuerdo de nivel de servicio sobre la rapidez con que se cambian las revisiones. Casi nada está en la cola durante más de 48 horas y la mayoría de las revisiones se realizan en menos de 24 horas.

Adrian J. Moreno
fuente
1

La compañía para la que trabajo requiere que otros desarrolladores revisen todo el código antes de que se comprometa. Los miembros de mi equipo a menudo se sienten frustrados ...

A menos que esté haciendo un software de misión crítica o parches críticos para el código de candidato de lanzamiento de producción, no hay razones convincentes para seguir rígidamente con un tipo particular de revisiones de código.

  • La idea detrás del requisito de su empresa parece razonable en la superficie (código 100% revisado), pero los medios que decidieron usar son contraproducentes, porque, como usted señala, esto lleva a que los desarrolladores se sientan frustrados.

Caminando en su lugar, primero señalaría a la gerencia que las revisiones de código posteriores a la confirmación se consideran tan respetables como las anteriores . Estos términos se pueden buscar en la web; si es necesario, encuentre referencias para hacer una copia de seguridad de esto. Uno no necesariamente necesita revisiones previas a la confirmación para obtener el código revisado al 100%.

Con base en lo anterior, a continuación les sugiero que abandonen la actitud de microgestión y permitan que los desarrolladores prueben de la manera que les resulte más conveniente. Las revisiones previas o posteriores al compromiso son las mejores para que los programadores elijan. Si la empresa sabe mejor que los programadores, ¿por qué no escriben ellos mismos el código?

mosquito
fuente
1
"Si la empresa sabe mejor que los programadores, ¿por qué no escriben ellos mismos el código?": ¡Muy buen comentario! Pero espero que los gerentes de desarrollo sean antiguos desarrolladores.
Giorgio
3
Post-commit perjudica la calidad de su código terriblemente en mi experiencia. Los programadores son perezosos y sentirán que han terminado si se puede comprometer: "sí, bueno, no es perfecto, pero a quién diablos le importa, ¿qué es perfecto en la vida? Hace el trabajo, ¿no? " la única buena respuesta es NO, y tal vez los gerentes tengan una imagen más realista de los programadores que la que tienen sobre sí mismos, por eso requieren revisiones de código previas (o al menos, previas a la fusión).
Aadaam
@Aadaam su experiencia definitivamente difiere de la mía, no solo con respecto a los compromisos posteriores, sino también (y especialmente) parte de "Los programadores son flojos ..." En cuanto a los gerentes que tienen una imagen más realista, estoy de acuerdo en que ese era el caso en general mis equipos solo no llevó a los gerentes con los que solía trabajar a ideas sin sentido sobre qué tipo de revisión forzar
mosto
Bueno, trabajé en outsourcing. En el outsourcing, la mayoría de los programadores no participan porque la programación es divertida, sino porque la programación tiene la mejor relación trabajo / salario, con tasas mucho más altas que cualquier otra cosa ... lo odian como cualquier otro trabajo ... y ellos trate de hacer todo lo posible para "optimizar" aún más esta relación, si sabe a lo que me refiero ...
Aadaam
1

Debe abordar una serie de cuestiones: debe ganarse los corazones y las mentes y debe asegurarse de que haya tiempo disponible para las revisiones de códigos.

La segunda parte es probablemente la más fácil: usted acepta (colectivamente y eso tiene que incluir la administración) que lo primero que hace un desarrollador cada mañana son sus revisiones de código: esto es simple, comprensible, efectivo y le da un buen palo claro para vencer a las personas con si no cumplen Mejor, no estás interrumpiendo nada, no les estás pidiendo que dejen de trabajar en su código, no les estás pidiendo a las personas que incluyan algo en su lista de tareas pendientes ...

La primera parte es el verdadero problema: los participantes en el proceso de revisión tienen que ver que tiene valor; de lo contrario, nunca harán una revisión del código (que se percibe como que no tiene valor) cuando podrían escribir código o corregir errores (que seguramente es más importante ...?).

Si puede unir los dos, en primer lugar, asegurarse de que todos crean (o entiendan) que hay un valor en las revisiones de código, en el más básico, debería significar menos errores, lo que significa más código nuevo, que generalmente es más divertido, y luego organizar cosas para que haya un espacio libre en el cronograma para que se realicen las revisiones del código, entonces espero que sucedan cosas buenas ... se convertirá en parte de la cultura.

Una vez que es parte de la cultura, puede que ya no sea necesario decir "lo primero todos los días", pero habiendo dicho que creo que encaja bien en el patrón, uno probablemente quiera un desarrollador para trabajar.

Murph
fuente
No se puede realmente de acuerdo en que "a primera hora todos los días" regla en el primer lugar. Si alguien encuentra un error que le cuesta a la compañía X dólares por hora (o aumenta el riesgo de perder un plazo importante en X puntos porcentuales por hora), y sucede que lo hacen cinco minutos antes de que ingrese, entonces la revisión del código no es suya. máxima prioridad. Básicamente, el problema es el choque entre el deseo de establecer reglas simples, frente al hecho de que las prioridades no siempre son simples. El riesgo es que todos estén de acuerdo de todo corazón con la regla, y dentro de 24 horas encontrarán alguna razón por la cual hoy es una excepción a la regla.
Steve Jessop
Y la solución es complicada, pero IME tiene que encontrar suficiente "espacio" para introducir una nueva práctica de trabajo que requiera mucho tiempo pero que valga la pena. Esto requiere previsión para identificar el tiempo de inactividad, la voluntad de dejar pasar los plazos como el precio de introducir un cambio que valga la pena, o ambos. TANSTAAFL. Como usted dice, una vez que todos se hayan acostumbrado al patrón, pueden hacer excepciones. Esperemos que lo hagan basándose en una apreciación adecuada del valor del patrón general.
Steve Jessop
Digo "dejar pasar los plazos", debería haber dicho "mover plazos". "deslizamiento" implica moverlos después de haberlos cometido, es decir, fallar, pero no tiene por qué ser así. En cambio, podría planificar un período de productividad ligeramente reducida debido a la nueva regla inflexible (y las ineficiencias inevitables causadas por las personas que intentan seguir cualquier proceso nuevo; si está haciendo la revisión del código a primera hora, se perderá el scrum de la mañana) reunión en días, la revisión del código lleva demasiado tiempo, o cualquier inconveniente único que su organización pueda incluir en la mezcla). Si es una buena regla, pronto superarás el punto de partida.
Steve Jessop
@SteveJessop, por supuesto, puedes estar de acuerdo con eso. Y, por supuesto, habrá excepciones (creo que la observación scrum es una tontería, especialmente porque la respuesta es obvia (-:). Creo que la clave es que no hay una "solución única para todos") propuso algo que es simple y fácil de entender y es relativamente difícil de modificar (según el entorno)
Murph
1

En la mayoría de las empresas para las que he trabajado, tiene 3 días para completar una revisión. No es aceptable no hacer las revisiones. Es parte de tu trabajo. Si no realiza revisiones decentes a tiempo, esto afectará su evaluación del desempeño. Y sí, las revisiones siempre parecen suceder en los momentos más inoportunos. Lástima, aprenda a incluir el tiempo de revisión en sus estimaciones. De todos modos, si la gerencia realmente cree que las revisiones son importantes (es decir, exigen que se revise todo el código), impulsarían una política similar. Además, si las personas no completan la revisión en el tiempo asignado, eso es como su aceptación del material.

Remojar
fuente
0

Considere usar una herramienta como Review Board . Es muy útil, especialmente para revisiones largas.

Puede cargar sus diferencias y esperar hasta que un revisor haya finalizado su revisión. Si tiene revisiones abiertas que le impiden continuar su trabajo, puede informar esto durante sus reuniones diarias (su equipo quiere que se registren nuevas funciones para que puedan probarse lo antes posible, ¿no?).

Giorgio
fuente
0

Algunos puntos para agregar que no están en las otras respuestas.

El código a revisar debe ser registrado

  • para que esté revisando una versión estable.
  • Puede estar en la rama de desarrollo principal si una versión está lo suficientemente lejos
  • Puede estar en la rama si hay una buena razón para no contaminar

Las tareas de bloqueo tienen prioridad, por lo tanto, las revisiones de código deben tener prioridad sobre otro trabajo (pero tratando de no interrumpir su flujo). Como desarrollador, debe querer que otros revisen su código (ya que su objetivo es mejorarlo). En ese conocimiento, debe realizar revisiones para otros de inmediato.

Las preguntas más difíciles son cuándo y cómo hacer bien las revisiones de código.

Una regla que nos ha funcionado en el momento es que el código compartido debe revisarse ya que tiene un impacto más amplio, mientras que en el código para una sola aplicación (especialmente dado que estamos usando un desarrollo basado en pruebas) es menos importante.

Bruce Adams
fuente