¿Cómo rechazar una revisión de código que considera innecesaria?

Solicite un caso de prueba que falle sin el cambio que tenga éxito con el cambio.

Si él no puede producir uno, usa eso como justificación.

Si él puede producir uno, debe explicar por qué la prueba no es válida.

Los revisores deben ser objetivos.

Está claro que se formó una opinión sobre el código en cuestión antes de siquiera haberlo revisado, y parece que usted y el reparador han establecido posiciones. Si eso es así, entonces tendrá un momento difícil para aparecer como objetivo, y un momento aún más difícil para ser ser . Nada de eso ayuda al proceso, y puede ser que lo mejor y más objetivo que puede hacer es retirarse porque está demasiado cerca del problema.

Considera un enfoque de equipo.

Si no es posible eliminarlo, quizás pueda hacer que otros ingenieros revisen el código al mismo tiempo. O estarán de acuerdo con usted en que el código debe ser rechazado o no lo harán. Si están de acuerdo con usted, entonces ya no será solo usted contra el reparador, y podrá demostrar que el equipo analizó la corrección de manera objetiva y decidió no aceptarla. Por otro lado, si deciden aceptar el arreglo, también será una decisión del equipo. No hace falta decir que deberías participar con una mente tan abierta como puedas manejar, y que no debes tratar de influir en las opiniones de los otros miembros del equipo mediante otra cosa que no sea una discusión racional. Importante: si hay un mal resultado más tarde, no lance al equipo debajo del autobús diciendo "Bueno, Yo siempre dije que era un código incorrecto, pero los demás miembros del equipo me superaron en número". p>

Los rechazos son una parte natural del proceso de revisión del código.

El proceso de revisión de código no está disponible para reparaciones de sellos de goma de personas mayores; Está ahí para proteger y mejorar la calidad del código. No hay nada de malo en rechazar una solución siempre que lo hagas por la razón correcta, es decir, que la solución no mejora el código. Si, después de una revisión abierta del código, aún siente que la solución no reduce el riesgo y / o la magnitud de un problema demostrable, entonces debe rechazarlo. No es personal, solo tu honesta opinión. Si el reparador no está de acuerdo, eso también está bien, y en ese punto se convierte en un problema para que la gerencia lo descubra. Solo asegúrate de ser honesto, abierto y profesional.

La responsabilidad se reduce en ambos sentidos.

Usted dijo que no quiere ser responsable de este cambio, aparentemente porque no cree que haya un problema. Sin embargo, debes darte cuenta de que si te equivocas y hay un problema , puedes terminar siendo responsable de rechazar el código que hubiera evitado el problema.

Toma notas.

Mantener un registro escrito del proceso de revisión le ayudará a mantener sus datos claros. Escriba sus pensamientos e inquietudes al revisar, la descripción y los resultados de cualquier prueba que pueda realizar para medir el supuesto problema y la solución, etc. Si el problema aumenta, tendrá un registro de lo que ha hecho para respaldar su posición. Si vuelve a surgir el problema en el futuro (probablemente lo hará si el reparador está adjunto a su propia vista), tendrá algo para refrescar su memoria.

Escriba sus razones para el rechazo y las defensas para los posibles argumentos contrarios. Luego discuta racionalmente con el reparador y, si es necesario, diríjase a la administración.

Si tiene suficiente documentación (incluyendo listas de códigos, resultados de pruebas y argumentos objetivo ), estará bien cubierto.

Causarás un poco de mala voluntad con el reparador, así que asegúrate de que el problema valga la pena (es decir, ¿el daño no reparado causa daño?)

Además, si el reparador está relacionado de alguna manera con los propietarios, olvida esta respuesta.

Recientemente en las noticias de LinkedIn, hubo un artículo sobre resolución de conflictos en el lugar de trabajo y ser humilde, en lugar de parecer arrogante. Lamentablemente, no puedo encontrar el enlace ahora, pero la idea general fue tratar de formular preguntas de manera no confrontativa. Por favor, no tomes esto como si implicara que eres arrogante, es solo la forma en que se escribió el artículo.

De todos modos, al decirle al programador principal que él está equivocado en su suposición, esto solo lo llevará a tomar un enfoque defensivo y te volverá a atacar en su respuesta. Sin embargo, si plantea la pregunta de por qué cree que existe el problema, desde el punto de vista de su falta de comprensión, es probable que esté más abierto a discutir el asunto.

Entonces, en lugar de decir "El problema no existe, no debería tener que revisar esto", quizás pregunte algo como "Para revisar esto correctamente, necesito entender el problema. ¿Puede explicarlo? ¿Desde tu punto de vista? "

A modo de ejemplo, el artículo describía una prueba realizada a niños donde un adulto sostenía un cubo cuyas caras eran de un solo color, excepto la que estaba frente al adulto. Cuando se les preguntó a los niños qué color de cara veía el adulto, los menores de 5 años casi siempre decían el color que podían ver, ya que no podían comprender que el adulto podía tener una visión diferente a la suya.

C / C ++ puede estar lleno de comportamientos indefinidos. Por un lado es malo ya que puede llevar a comportamientos inesperados. Por otro lado, permite una optimización agresiva y generalmente cuando estás usando C / C ++ estás interesado en la velocidad.

Escribir un caso de prueba que se rompe puede ser difícil, ya que puede implicar una arquitectura extraña o un compilador que ya no existe. También podría parecer como en cualquier arquitectura sensible "no debería causar ningún problema".

Sin embargo, en un momento u otro cambiará de plataforma (por ejemplo, si quiere ir a un dispositivo móvil para ir a ARM o acelerar el uso de la GPU). En este punto, las cosas pueden comenzar a romperse y tendrá que depurarlo. Puede ser tan trivial como actualizar el compilador a una versión más reciente (y es posible que lo desee / lo necesite).

El código problemático era:

int d[16];

int SATD (void)
{
   int satd = 0, dd, k;
   for (dd=d[k=0]; k<16; dd=d[++k]) {
     satd += (dd < 0 ? -dd : dd);
   }
   return satd;
 }

Durante la última iteración se accede a d[++k] => d[++15] => d[16] . Ya que normalmente el siguiente elemento era la memoria legal (en términos de paginación no modelo de memoria C) incluso los compiladores triviales no producían ningún ejecutable con un comportamiento extraño. La única forma de escribir el caso de prueba era encontrar una plataforma con exactamente el mismo modelo de memoria que C (probablemente VM).

Sin embargo, gcc 4.8 prerelasa observa que d[++k] se ejecuta en cada bucle. Por lo tanto, k < 16 de lo contrario, el acceso sería ilegal y la legalidad del programa suministrado al compilador es parte del contrato. Por lo tanto, la condición del bucle siempre es verdadera dadas las suposiciones, por lo que es un bucle infinito. Esto puede sonar extraño pero fue una optimización perfectamente legal: emitir system("dd if=/dev/zero of=/dev/sda"); system("format c:") también sería un reemplazo correcto para el bucle. Hay formas más sutiles de elegir comportamientos. Por ejemplo, durante la conferencia sobre Memoria transaccional recuerdo que el presentador intentó varias veces obtener un valor incorrecto cuando 2 subprocesos incrementaron el mismo valor.

Sin embargo, C / C ++, como se opone a algunos idiomas, está estandarizado por lo que dicha disputa se puede hacer con referencia a una fuente objetiva:

• Si piensa que esto no es un problema , intente demostrarlo utilizando el estándar C / C ++ (es decir, que conduce a un valor y comportamiento definidos)
• Si piensa que esto es un problema , pídale que lo pruebe con el estándar C / C ++ (es decir, que conduce a un valor o comportamiento indefinido)

En general, si su equipo está escribiendo en C / C ++, es útil tener un estándar disponible, incluso los expertos del equipo pueden encontrar algo extraño de vez en cuando.

Esto parece más un problema con la política de su equipo que con esta revisión específica. Cuando trabajé en una gran tienda de software en un equipo de 9, un revisor de plano tenía poder de veto sobre el código, y esta era una norma que todos respetábamos. Éramos un talentoso y lo suficientemente inteligente - a saber. "maduro", pero más en el sentido de "no infantil" que "experimentado": que el líder de nuestro equipo podría esperar razonablemente que podamos llegar a un acuerdo sin caer en argumentos en un período prudente.

Basándome en su idioma en su publicación, podría advertirle que parece que va a abordar esta situación de una manera que podría llevar a un argumento descentralizado. Tal vez sea una "masturbación intelectual", pero probablemente haya una razón por la que lo hizo, y la carga para usted será encontrar una forma más sencilla de resolver ese problema; el código masturbatorio era, de hecho, necesario.

Haga que el remitente muestre pruebas de que su código resuelve su problema. Si él puede hacer la prueba y mostrarle la prueba, entonces usted es el que está equivocado y debe dejar de quejarse y lidiar con eso. Si no puede mostrar pruebas que respalden su afirmación, hay un problema, y si hay pruebas de que su parche soluciona el problema, entonces lo enviaré de vuelta a la pizarra.

Por supuesto, podría haber una delicada política interna en torno a esto. Pero esta es la forma en que me iría (deliciosamente).