Estoy en la siguiente situación. Pequeño equipo dentro de una gran unidad con muchos equipos. Hay un proceso oficial de revisión del código, pero para muchos desarrolladores es más una formalidad, por lo que es raro ver discusiones o revisiones serias.
Dentro de mi equipo me encontré con un desarrollador que no seguía la rutina y se comprometía directamente con la rama de desarrollo. Abrí el tema de las revisiones de código, luego comenzamos a hacer solicitudes de incorporación de cambios. Mis solicitudes de incorporación de cambios se aprobaron automáticamente. Sin embargo, era obvio que no los estaba leyendo, solo presionando el botón.
Al mismo tiempo, parece que se lo está tomando un poco personal cuando estoy haciendo las reseñas. Para ser honesto, generalmente hago revisiones de código bastante exhaustivas. Me lo tomo en serio. He estado haciendo revisiones de código a otros desarrolladores en la empresa para la que trabajo, es solo este caso en el que el desarrollador parece estar extremadamente a la defensiva. El problema es que trabajamos muy cerca.
¿Debería dejar de hacer revisiones de código? ¿Cómo abordar esta situación de la mejor manera?
Solo un nuevo ejemplo para entender el punto. El desarrollador escribe una clase que parece algo entre Factory y Container, luego la llama SomethingBuilder donde la parte Something no está relacionada con el objeto finalmente construido. Por supuesto, comento esto y digamos que se lo toma como algo personal. ¿Qué se supone que debo hacer aquí? Parece que al líder técnico del equipo no le importa. Lo defiende mucho en realidad. Dice que cada uno tiene su propio estilo. No estoy seguro de qué tipo de estilo es llamar Factory a Builder, pero no importa. ¿Es la evasión la mejor estrategia aquí, especialmente cuando el Tech Lead muestra compasión por el tipo?
Creo que si su líder tecnológico definitivamente no lo respalda, entonces tendrá que modificar su comportamiento.
Esta es la desafortunada realidad de la vida en una empresa.
Lo mejor que puede hacer es pedirle a su líder técnico que le dé pautas sobre cómo deberían ser sus revisiones de código y mantenerse dentro de esas pautas. Luego, si el desarrollador está respaldado por algo que usted dijo en una revisión, entonces puede dirigirse a su líder técnico y pedirle que lo respalde según sus pautas .
He estado en ambos lados de esta ecuación y aunque es difícil decir exactamente lo que está sucediendo sin conocer a las personas involucradas o cómo ven las cosas, parece que debes tener un poco más de tacto aquí. Si tuviera que adivinar (y es solo una suposición) tu compañero de trabajo probablemente se esté poniendo a la defensiva porque se siente atacado. Para ti es una parte impersonal del trabajo, para ellos eres un idiota que se enfada con ellos por no hacerlo de la manera que crees que debería hacerse.
Sin embargo, podría estar fuera de lugar aquí. Lo primero que debes hacer es hablar con ellos al respecto. Esta conversación no necesita ser súper formal, pero debe ser privada. Hágales saber que está tratando de ayudarlos a mejorar y que sus críticas no son personales. Además, no te centres solo en los aspectos negativos. Si tienen alguna respuesta a esto, entonces escúchelos y trate de entender su punto de vista. Sin embargo, incluso si no sale nada de esta conversación, hay cosas que puedes hacer.
¿Tiene su empresa algún estándar de código formal? ¿Están documentados estos estándares? Si es así, entonces tu vida se volvió mucho más fácil. Cada vez que vea algo que viole uno de estos estándares, cortésmente señale la violación. Si se enfada, incluso puedes decir "Oye, estoy de acuerdo, es un estándar estúpido, pero yo no hago las reglas". Incluso puedes hacer que parezca que le estás cuidando las espaldas diciendo "oye amigo, no quiero que te metas en problemas, probablemente deberías cambiar esto para que coincida con el estándar".
La otra gran cosa que puede hacer es explicar por qué se debe cambiar algo. No seas como "oye, cambia X por Y". Explíquele que X provoca errores en el escenario realista Z o que Y es más limpio y hace que el código sea más fácil de mantener. Nuevamente, intente expresar esto de tal manera que su compañero de trabajo sienta que está tratando activamente de ayudarlo a mejorar en lugar de simplemente reprenderlo porque hizo las cosas a su manera en lugar de a usted.
Señale las cosas buenas en su código, así como las malas. Si hicieron algo de manera inteligente, dígales que le gustó la forma en que lo hicieron. Esto ayudará a evitar que se sientan como si estuvieras tratando de atraparlos.
El desarrollador escribe una clase que parece algo entre Factory y Container, luego la llama SomethingBuilder donde la parte Something no está relacionada con el objeto finalmente construido. Por supuesto, comento esto y digamos que se lo toma como algo personal.
Mucho depende de cómo hayas comentado esto. Comparar:
Esta clase parece algo entre Factory y Container, y se llama SomethingBuilder, que no está realmente relacionado con el objeto finalmente construido.
a:
Me pregunto si valdría la pena el esfuerzo de dividir esto en una Fábrica y un Contenedor de esta manera:
[.. pequeña cantidad de pseudocódigo para demostrar lo que quieres decir..]
Creo que esto nos dará [ventaja concreta X].
¿Qué piensas?
Además, parece ser que Algo realmente no transmite la intención, ya que es más una Otra Cosa. ¿Quizás cambiarle el nombre a Otra cosa?
El primer ejemplo, que se parafrasea a partir de su pregunta, equivale a poco más que quejarse del código y decir "su código apesta". No creo que sea particularmente extraño tomar comentarios como ese personalmente.
El segundo ejemplo ofrece una alternativa, y también lo expresa como una sugerencia y solicita comentarios, en lugar de simplemente decirle a su compañero de trabajo qué hacer.
Dice que cada uno tiene su propio estilo. No estoy seguro de qué tipo de estilo es llamar Factory a Builder, pero no importa.
Esto suena desdeñoso con respecto a las opiniones y convenciones de su compañero de trabajo, y como si ni siquiera estuviera dispuesto a tomárselo en serio. En los programas del mundo real, las cosas a menudo no están tan bien organizadas; puede haber una buena razón para escribirlo como lo hizo él, pero si tu fraseo no comunica que estás abierto a alternativas, entonces no vas a tener esa conversación.
Tal vez su compañero de trabajo solo esté escribiendo un código tonto; No sé. Pero decirle "estás escribiendo un código tonto" no es una forma muy efectiva de cambiar eso. Pregunte por qué lo hizo de la manera que lo hizo. Escucha. Proporcione alternativas significativas y explique por qué cree que es mejor; dale espacio para contemplarlo y experimentar con él.
No esperes que cambie simplemente lanzándole las definiciones del diccionario de "Fábrica" o "Constructor"; explicar cómo mejorará las cosas en términos objetivos. A menudo se usa "más elegante", pero no es un buen argumento. Si tiene dificultades para encontrar mejores argumentos que "más elegantes", entonces tal vez ... ¿realmente no importa tanto?
Para ser honesto, generalmente hago revisiones de código bastante exhaustivas.
Algunas revisiones de código pueden ser "demasiado exhaustivas", comentando cosas que no importan mucho.
En mis propias revisiones de código, siempre me detengo y pienso antes de dejar un comentario: "¿Esto realmente mejorará objetivamente la calidad del código, o es solo mi preferencia personal?" No pocas veces, es solo mi preferencia personal; así que lo dejé pasar.
He visto a personas comentar sobre algunas cosas subjetivas muy pequeñas. Está bien comentar un error de ortografía/error tipográfico menor en un comentario, ya que es objetivamente incorrecto, pero creo que es importante reconocer que la programación es un oficio muy subjetivo y que la objetividad es difícil de conseguir (por eso los programadores están constantemente peleándose por todo ).
Aquí me parece que usted no es el hombre alto en el tótem, por así decirlo. Como tal, en realidad solo hay un número limitado de cosas que puedes hacer, y parece que las has hecho:
1) Si el equipo no está realizando un proceso de PR/RC, debe presionar para instituirlo (lo hizo).
2) Si las personas están dando el visto bueno a las relaciones públicas, debe instarlas a que se detengan (lo hizo).
3) Debes ser el ejemplo y cuando alguien te envíe un mal PR, debes comentarlo por todas partes y explicar qué es lo que está mal (tú lo hiciste).
4) Si las personas no están trabajando como deberían, debe informarle al líder técnico (usted lo hizo).
En este punto, es responsabilidad del líder técnico manejar la situación como quiera. En este caso, la cultura viene desde arriba; el desarrollador más experimentado debe explicar al resto del equipo por qué el proceso de relaciones públicas es importante o imponer que deben hacerlo correctamente incluso si no entienden por qué (lo primero es óptimo, pero lo segundo también funciona). Sin embargo, si el líder técnico no entiende por qué es un proceso valioso o no quiere esforzarse para asegurarse de que se haga correctamente, no hay mucho que pueda hacer.
En cuanto a lo que debe hacer en el futuro, por mucho que me duela decirlo, debe mencionar los problemas, pero si las personas que reciben el problema piensan que no es un problema, entonces debe aprobarlos. Estás en peligro de pasar de "el responsable" a "el anal-retentivo", y no quieres la reputación de ser difícil de trabajar, no vale la pena. También recomendaría buscar un nuevo puesto; no quieres trabajar con malos desarrolladores que te van a derribar con ellos. Deje que se queden en su barco que se hunde, y usted debe adoptar su enfoque responsable del desarrollo en una empresa donde se valore tal habilidad.
Yo diría que se apeguen a las armas aquí. Su revisión de código suena razonable. Si su líder técnico permite convenciones de nomenclatura deficientes, causará confusión entre los desarrolladores que no escribieron esta clase. Las convenciones de nomenclatura son útiles para muchas cosas y la convención de nomenclatura estándar es el camino a seguir.
Aquí hay algunas ideas prácticas para resolver esto.
Intente separar los comentarios de revisión de código en puntos objetivos y subjetivos. Los puntos objetivos incluyen observaciones como "aquí hay un error potencial" o "esto no cumple con los requisitos". Los comentarios subjetivos incluyen "hay una mejor manera de hacer esto" o "esto sería mejor llamarlo X". A menudo es útil ser explícito en los comentarios sobre la diferencia: "En mi opinión..." / "Considera intentarlo...". Creo que esto ayuda a reducir la actitud defensiva.
Acepte que, a menos que esté en una posición más alta, solo puede sugerir mejoras donde los puntos son subjetivos. Tienes que aceptar que habrá algunas diferencias de estilo en el equipo, por mucho que te parezca obvio que es preferible una forma.
Si la discusión parece acalorada, a menudo es útil chatear en persona en lugar de a través de cualquier herramienta de relaciones públicas que esté utilizando. Puede tener una discusión más profunda y tener más tacto con el lenguaje corporal apropiado. Es posible que aún no estén de acuerdo, pero podría ser más fácil ver el punto de vista del otro.
Para cuestiones como las convenciones de nomenclatura, a menudo es útil obtener un acuerdo del equipo sobre cómo llamar a los módulos de tipo X. Slack u otra herramienta de comunicación interna a menudo ofrece una opción de encuesta para que los miembros del equipo voten sobre las convenciones de nomenclatura preferidas. Una vez que tenga el acuerdo del equipo, puede señalarlo en futuras revisiones de código. Ofrezca configurar una página wiki con las decisiones de su equipo.
Con respecto a las preguntas de mayor nivel sobre cómo aprobar los RP o cómo resolver conflictos sobre los RP, estos son buenos temas para plantear en una retrospectiva (suponiendo que su equipo siga algún tipo de proceso Agile).
Su compañero de trabajo parece desaprobar todo el proceso de revisión. Su líder técnico es escéptico. Estos son los dos temas que debe abordar, comenzando con el que tiene más que decir sobre el proceso.
El líder técnico puede ser escéptico por su forma de hacer revisiones o con revisiones en absoluto. Necesitas averiguar por qué está defendiendo a tu compañero de trabajo. Estar abierto a la crítica. Quizás eres demasiado formal. Tal vez la forma en que das retroalimentación no parece lo suficientemente amable.
Entonces debería convencerle de que, sin embargo, las revisiones son necesarias para garantizar la calidad del código y el nivel del equipo. Sugiera que el equipo puede crecer en el futuro y que estos problemas deben abordarse cuando sea menos doloroso.
Su firme aprobación es clave para el éxito de las revisiones. Si él no está de acuerdo, simplemente puede dejar pasar el proceso de revisión, pero eso no significa que no tenga un problema con el compañero de trabajo.
Los desarrolladores en general (yo definitivamente incluido) tienden a poner mucho ego en su trabajo. El problema con las reseñas no es nuevo, algunas personas lo toman personalmente como una crítica a su trabajo. Esto es fundamental, porque los desarrolladores son caros y la buena moral es importante.
Es por eso que, sin importar si su líder técnico aprueba las revisiones, debe enfatizar el hecho de que, a pesar de que las revisiones fueron difíciles, cree que está haciendo un buen trabajo y no debería sentirse inseguro. Compruebe si él no está programando con prisa también, recuérdele si es necesario su preferencia por la calidad sobre la cantidad.
No estoy seguro de que haya mucho que hacer aquí. Si el ejemplo más notorio que tiene es el nombre de una clase, y ni siquiera está tan mal llamado, sinceramente, no estoy seguro de la diferencia entre "constructor" y "fábrica", entonces creo que es posible que desee reconsiderar lo que re revisar en una revisión de código.
Mira, una reseña no siempre tiene por qué tener un problema . Si el desarrollador coincide con las convenciones de nomenclatura del equipo, sean las que sean, no es necesario que haga comentarios sobre los nombres.
Si detecta, por ejemplo, una fuga de memoria o algún código extrañamente ineficaz, entonces sí, téngalo en cuenta. Pero no caigas en la trampa de "cuando me piden que comente algo, tengo que comentarlo de alguna manera". tu no Estás en la industria, el 75% es un esfuerzo encomiable, diablos, el 35% es un pase la mayoría de las veces.
Mientras las cosas funcionen y no funcionen mal y se haga a tiempo, bueno, ¿qué más quieres? No se le paga por líneas de código hermoso, no se le paga por líneas de código en absoluto, se le paga por resultados prácticos .
Parece que está agregando trabajo ocupado al equipo, tenga cuidado. Mírelo de esta manera: si estuviera dirigiendo una empresa, ¿querría un código hermoso que no hiciera nada, o un código de basura que apenas se escalara, pero que pudiera vender?
Si cree que hay alguien vivo que dirige una empresa que quiere la primera, es posible que tenga razón, pero no estarán dirigiendo esa empresa por mucho tiempo.
Una discusión sobre la denominación
Las convenciones de nombres son estúpidas. Las palabras cambian de significado con el tiempo, y tratar de unir el nombre a la función no tiene sentido cuando la función está ahí y se puede leer .
Cualquiera puede leer el código para entender lo que hace. Pero todos tendrán una idea diferente de lo que significan las palabras, por lo que insistir en que las palabras coincidan con el significado para su comprensión de las palabras es una premisa incumplida, a menos que sea la única persona que leerá el código, lo cual no es.
Peor aún, desea que los nombres coincidan con el funcionamiento interno cuando, en realidad, los nombres deberían hacer una de (o mejor, ambas) dos cosas:
Describir el funcionamiento interno no tiene sentido porque, de nuevo, el código se puede leer. Capturar la lógica empresarial es mucho, mucho más importante. A ápice:
saveDataInAthreadSafeAndDataConsistentManner(){...}
no es útil Pero
saveAddressDetailsAsTheresAConcurrentProblemWhenSavedWithEverythingElse()
bueno, ese es un nombre de método útil.
Pero al final del día, el código de trabajo >> código bien nombrado que no funciona, y produjo rápidamente un código de trabajo >> produjo lentamente un código de trabajo bien nombrado.
otro pensamiento
Nunca parezca que usted es la persona en el equipo que hace cumplir los estándares que ralentizan al equipo; esa no es una buena apariencia. Si el líder de su equipo estuvo de acuerdo con usted, estaría bien continuar con esto, pero el TL no lo está, por lo que ahora parece que solo se está preocupando por las convenciones de nombres. Yo no haría esto.
Es triste decirlo, pero siente que necesita dejar ese equipo, tal vez dejar esa empresa. No eres un buen ajuste cultural. Tanto el líder de su equipo como quien ascendió a su líder de equipo han elegido recompensar un patrón particular de comportamiento. Las posibilidades de que esas personas se vayan y que toda la cultura cambie son muy pequeñas.
En lugar de recompensarlo por preocuparse, el líder de su equipo ha sido irrespetuoso y desdeñoso con sus intentos de mejorar la calidad de la mano de obra dentro de su equipo. Esto lo coloca en una posición políticamente débil y fácil de culpar por ralentizar las cosas, cuando de hecho el código está "escrito una vez y leído muchas veces", fácil de leer, comprender y mantener significa un camino más rápido hacia un producto libre de errores. Según su propia descripción, el estilo de código de su colega lo retrasó 30 minutos para averiguar exactamente lo que hizo una sola clase. Seguramente tal descripción debería haber sido parte del nombre de la clase.
Además, su cadena de gestión ha ignorado las matemáticas básicas y la psicología básica.
Para las matemáticas, si un módulo tiene un 99 % de posibilidades de no tener errores (es decir, no usar suficientes pruebas unitarias), entonces puede calcular (1-p)^n que 100 módulos juntos tienen un 37 % de posibilidades de comportamiento libre de errores . Las probabilidades van en tu contra muy rápidamente. Mejorar la "calidad" al 99,5 % mejora el sistema a poco más del 60 %.
Para la psicología, ¿quién quiere trabajar para o con personas a las que no les importa?
bharal
Pesho
Pesho
bharal
Pesho
Pesho
Bernardo Barker
Bernardo Barker
Pesho
Pesho
Dan
Konrad
isherwood
Brandín