Revisión de código: malos consejos para contribuidor y revisor

Hola Me llamo Nikolai Izhikov. En esta publicación quiero hablar sobre un elemento importante de interacción que encontramos en el proceso de desarrollo de software, especialmente en código abierto. Este es un tutorial y una revisión de código.


Daré consejos perjudiciales sobre cómo crear mi función y la fusionaré en el asistente de proyecto de código abierto en el contexto de diferencias culturales, temporales, conceptuales y de otro tipo entre los miembros de la comunidad. Esta suele ser una pregunta difícil, especialmente para aquellos que recién están comenzando en código abierto.

Comencemos con una peque√Īa introducci√≥n. Cuando se comunique con personas, especialmente en el chat o por correo, especialmente en ingl√©s, siempre tenga en cuenta que su mensaje se puede percibir de manera completamente diferente de lo que esperaba. Qui√©n leer√° la carta es desconocido. Puede ser hind√ļ, ingl√©s o simplemente una persona somnolienta. Siempre habr√° diferencias en la comprensi√≥n de palabras espec√≠ficas, y sin entrar en detalles t√©cnicos, le dir√© c√≥mo minimizarlas.

Advertencia: la historia contiene sarcasmo.



Malos consejos para un contribuyente


No discuta una nueva característica con nadie


Si desea implementar una nueva funci√≥n, en ning√ļn caso discuta su revisi√≥n con nadie. ¬°Alguien puede espiar y hacerlo antes que t√ļ! O, habiendo escuchado los detalles, pueden decirte que no entendiste algo para criticar tu idea. La cr√≠tica duele mucho. Tal vez una caracter√≠stica como la suya ya ha sido discutida, y necesita otra ... En general, en ning√ļn caso diga a nadie lo que quiere hacer. Nunca



Nunca haga documentación técnica


Los encargados y los muchachos experimentados de la comunidad son muy aficionados a comprender el c√≥digo del parche recibido. Por lo tanto, en ning√ļn caso no haga documentaci√≥n t√©cnica. Wiki, Jira, discusi√≥n de la lista de correo: todo esto no tiene sentido. ¬°Y la mierda no es para ti!

Cuando envía un parche a [+5000, -3500] con una descripción de las mejoras en forma de "Mejoras en los métodos de fábrica y algunas otras mejoras", todos lo descubrirán por sí mismos y, al mismo tiempo, comprenderán lo bueno que es.



Nunca ejecutes pruebas


Cualquier prueba es peligrosa porque puede romper algo. Si a√ļn logr√≥ ejecutar las pruebas, no muestre sus resultados. ¬°Los errores en los resultados pueden conducir al rechazo del parche! Y definitivamente nunca escribas nuevas pruebas. Run All ser√° m√°s largo, el consumo de CPU aumentar√°. De todos modos, los buenos desarrolladores escriben c√≥digo sin errores: cualquier colega experimentado mirar√° sus parches y comprender√° que no hay errores all√≠.



Nunca lea cómo hacerlo


Al entrar en un proyecto de c√≥digo abierto, nunca lea ning√ļn tutorial. Est√°n escritos para tontos, ¬Ņverdad?
Dise√Īa tu c√≥digo a tu manera. De lo contrario, ¬Ņc√≥mo entender√°n todos que usted es un buen desarrollador y una persona creativa vers√°til con un sentido desarrollado de belleza?

Envíe parches ya que es conveniente para usted. Por ejemplo, el proyecto está vinculado a la infraestructura de GitHub: envíelo tal como se envía al kernel de Linux, justo en la carta. Incluso puede no en el archivo adjunto, sino directamente en el texto. ¡Después de todo, Linus Torvalds no aconsejará nada malo! Y si el proyecto se adopta de manera diferente, entonces estos son problemas del proyecto.



Si√©ntase libre de complicar la API p√ļblica


Al desarrollar la nueva API p√ļblica, intente que sea lo m√°s abstracta y compleja posible. Cualquier pregunta del usuario sobre el comportamiento no obvio de la API siempre se puede responder: ‚Äú¬ŅNo ha le√≠do el manual? ¬°P√°gina 42, todo est√° escrito claramente! ¬°L√©elo de nuevo! En proyectos serios, todo deber√≠a ser complicado. Tenemos un proyecto serio?



No aconsejar ni hablar sobre problemas.


No consulte con nadie y no le cuente a nadie sobre los problemas encontrados en el desarrollo. Es mucho más divertido entender una cosa. De todos modos, los buenos desarrolladores siempre tienen éxito la primera vez, no tienen problemas. Si otros descubren sus problemas, se volverán más inteligentes y escribirán código más rápido que usted. Esto no debería permitirse. Y, de hecho, no es costumbre hablar sobre sus problemas.

Tampoco vale la pena mencionar las limitaciones de la decisi√≥n final. Despu√©s de todo, se le puede pedir a la soluci√≥n que finalice. ¬ŅA qui√©n le importan las restricciones? Cuando un usuario comienza a introducir su producto en el producto y encuentra restricciones, estar√° m√°s interesado y ser√° m√°s divertido. √Čl vendr√° a ti y te pedir√° que lo termines. Y hasta este punto, en ning√ļn caso no le cuente sobre las restricciones, ¬Ņqu√© pasa si decide no implementar nada?

Un revisor muy bueno encontrar√° todo y le pedir√° detalles. En ning√ļn caso no le cuentes nada.



Si tiene preguntas, escriba a la lista de desarrolladores


Este consejo complementa el anterior. Es mejor no solo no decirle nada a nadie, sino también hacer preguntas principalmente en la lista de desarrolladores.

Aqu√≠ hay ejemplos de preguntas que a todos les gusta responder. Si escribe alg√ļn tipo de prueba, aseg√ļrese de preguntar: "¬ŅNecesita verificar la nula de esta colecci√≥n?". No tiene que resolverlo por su cuenta, siempre puede preguntar en la hoja de desarrollo. Despu√©s de todo, hay muchas personas que solo esperan que se les haga esa pregunta. Buscar√°n responderle m√°s r√°pido.

"¬ŅC√≥mo hago la tarea?" - Otra gran pregunta. En ning√ļn caso, no indique ning√ļn detalle: la ID de la tarea ser√° suficiente. Todos los que lo necesiten ver√°n por s√≠ mismos.

"¬ŅQu√© marco utilizar?" - Tambi√©n una muy buena pregunta. Siempre es interesante responderlo, y puedes debatir.



Solucione todos los problemas del proyecto en una solicitud de extracción


Este es mi consejo favorito. Solucione todos los problemas del proyecto en una solicitud de extracci√≥n, especialmente si est√° trabajando en una empresa. Solo hab√≠a tontos en el proyecto antes que t√ļ, escribieron mal el c√≥digo. Y escribes bien. En consecuencia, simplemente debe:

  • Refactorice todo el c√≥digo existente que no comprende;
  • cambiar el nombre de todas, en su opini√≥n, las variables nombradas incorrectamente;
  • arregla todos los comentarios existentes de javadoc.

En general, puede tomar y sacar algunas clases, fábricas, etc., hacer otras transformaciones para que el código sea mejor. Esto se verá especialmente impresionante en áreas que no son relevantes para su tarea. Para que pueda revelar más plenamente su potencial. ¡A la gloria de la OOP! Amén!



Solicite una revisión de código por adelantado


Cuando realiza una tarea y luego envía una solicitud de revisión de código, el proceso puede demorar un poco. Todos los expertos suelen estar ocupados. Aproveche el truco: cuando su tarea, como le parece, termine pronto, solicite una revisión por adelantado. Después de todo, no se verán de inmediato: hasta que tus manos te alcancen, puedes terminarlo todo.

Bueno, si el revisor de repente tiene tiempo, mientras la tarea a√ļn no se ha completado, entonces tuvo mala suerte. Explique la situaci√≥n, diga que ma√Īana todo estar√° listo. De esta forma, acelera el proceso (al menos mediante revisi√≥n) y se fusiona m√°s r√°pido.



Cansado de la tarea: suéltalo


Todos los que trabajan en c√≥digo abierto tienen mucho tiempo. No hay necesidad de terminar nada. Revisi√≥n del c√≥digo aprobado, comentarios recibidos. ¬ŅY por qu√© editar y traer algo para fusionar? Toma el siguiente rompecabezas y hazlo. Este es el camino hacia el √©xito! El revisor tiene mucho tiempo, ¬°y el pr√≥ximo parche se ver√° sin resultado!



El crítico es tu enemigo


¬ŅY qu√© m√°s nombrar a la persona que se interpone entre usted y el cometer en maestro? ¬°La cr√≠tica del c√≥digo es una cr√≠tica personal! ¬ŅQui√©n se cree que es? En comunicaci√≥n personal, "bombardea" tanto como sea posible! De lo contrario, ¬Ņc√≥mo sabe el revisor que le importa? ¬°Esta es la regla b√°sica del desarrollo!

Recomiendo este consejo en el desarrollo cotidiano. Esto ayuda a lograr el resultado y hacerlo bien.



Mal consejo para el revisor


No automatice las comprobaciones de rutina.


Si ha alcanzado el nivel en el proyecto cuando ya le est√°n enviando parches para su revisi√≥n, ¬°en ning√ļn caso automatice las comprobaciones de rutina! Es mucho m√°s divertido pasar un par de ciclos de revisi√≥n en soluci√≥n de problemas y discusi√≥n:

  • formato de c√≥digo;
  • nomenclatura variable;
  • comprueba si esas variables est√°n marcadas como finales, lo que puede marcarlas;
  • ... y todo lo dem√°s.

En ning√ļn caso se deben automatizar los controles de rutina. S√≠, y pruebas para conducir a la nada.



Nunca reveles todas las reglas del juego hasta el final.


Para adelantarse, siempre debe mantener un par de ases bajo la manga. No le digas a nadie sobre la necesidad de compatibilidad con versiones anteriores. Es mejor decir justo antes de la confirmación: "Aquí, de acuerdo con nuestras reglas, debe garantizarse la compatibilidad con versiones anteriores. Por favor corrija ". Esto será especialmente efectivo si ya ha revisado cinco veces. En el sexto todavía se puede decir: "No soy un experto, por lo que debe esperar la revisión del Sr. X, que volverá a mirar".

Tales comentarios, especialmente en la etapa final de la revisión, siempre agregan motivación a los contribuyentes.



Emociones, autoridades y no gracias.


Esta sugerencia se superpone con la √ļltima sugerencia de contribuyente. Escriba comentarios sobre la solicitud de extracci√≥n lo m√°s emocionalmente posible. Si en alg√ļn lugar no se marca nulo o la variable no se nombra de esa manera, agregue pasi√≥n a sus comentarios. Deja que vean que te importa.

Si surge una disputa, en ning√ļn caso dar argumentos t√©cnicos. Con argumentos t√©cnicos, puede resultar que est√°s equivocado. Consulte a las autoridades: este es el mejor argumento en la disputa, siempre ganar√°.

Si alguien revis√≥ tu opini√≥n, nunca deber√≠as decir gracias. ¬°A√ļn as√≠ quieren comprometerse con el c√≥digo abierto! ¬°En ning√ļn caso no tienes que agradecer, y entonces vendr√°n otra vez!



Ahora en serio: ¬Ņqu√© debe pensar al preparar y realizar una revisi√≥n de c√≥digo?


Espero que todos hayan entendido cómo realizar y aprobar la revisión. En cada uno de estos consejos, además del sarcasmo, hay dolor que a menudo ocurre en la práctica.



Ser√© el capit√°n de Evidence y le dir√© lo que realmente necesita pensar al preparar y realizar una revisi√≥n del c√≥digo. Las consideraciones se aplican tanto al que desarrolla la funci√≥n como al que la revisar√°. A√ļn as√≠, estas son dos caras de la misma moneda.
El consenso en la comunicación es, en primer lugar, alcanzable, y en segundo lugar, es necesario que el proyecto avance. Actualmente, pocos productos se pueden desarrollar solos. Por lo general, esto es trabajo en equipo.

Usa el sentido com√ļn


Este es el consejo m√°s importante. Use el sentido com√ļn, se dirige. Me parece que este consejo se aplica a todas las situaciones de la vida. Si est√° haciendo algo, considere si esto cumple con la simple regla del sentido com√ļn.

Supongamos ...


Esto es sobre cultura. He estado en varias grandes comunidades de código abierto. No sé si esto es parte de la mentalidad rusa, pero a menudo una persona que puede ser percibida como un jefe es percibida simultáneamente como una persona que accidentalmente cayó en su lugar. Se cree que te quiere mal, por defecto hay dudas sobre su profesionalidad. Por lo tanto, es muy importante en cualquier trabajo asumir, al menos por un segundo, que:

  • El revisor (colaborador, su jefe o colega) no es su enemigo . √Čl no quiere que seas malo. Esta es una suposici√≥n simple, pero a menudo no se hace. Le aconsejo que lo haga de todos modos.
  • La persona que escribe sus comentarios tambi√©n es un buen ingeniero. Usted, por supuesto, es bueno, ha hecho tal caracter√≠stica. Pero hay muchos otros buenos ingenieros en el mundo. Y el que te envi√≥ comentarios tambi√©n se aplica a ellos.
  • Esta persona tambi√©n quiere que su tarea se cumpla.

    Cuando pide algo, tiene alg√ļn tipo de motivaci√≥n. Lo hace por una raz√≥n. Especialmente si esta persona no es el primer d√≠a en el proyecto. Seguramente hay alguna raz√≥n. Puede preguntar por esta raz√≥n: ¬ŅPor qu√© necesita hacer exactamente eso? ¬ŅPor qu√© se necesita compatibilidad con versiones anteriores aqu√≠? Si hace preguntas simples con calma, razonablemente y escucha las respuestas, puede lograr m√°s.

¬ŅQu√© valor aportar√° el producto?


La revisi√≥n no es solo parches listos, sino tambi√©n mejoras del proyecto, correcciones. De hecho, la revisi√≥n del c√≥digo comienza en el momento en que solo est√° discutiendo su revisi√≥n. En este punto, preg√ļntese: ¬Ņqu√© valor aportar√° el producto al producto?

  • ¬ŅSer√° una nueva caracter√≠stica?
  • ¬ŅEs esta una mejora existente?
  • Extensi√≥n de una caracter√≠stica existente?
  • ¬ŅSer√° la refactorizaci√≥n de c√≥digo? No hay nada de malo en eso. Algunos son cr√≠ticos de la refactorizaci√≥n, pero es necesario. Y debe tener en cuenta que lo est√° haciendo, y no una nueva caracter√≠stica u otra cosa.
  • ¬ŅEst√° acelerando un proceso, mejorando el rendimiento?
  • ¬ŅEs esto una correcci√≥n de errores?

Hay otras opciones En cualquier caso, al comenzar a desarrollar algo, para resolver un problema, debe comprender qué valor agregará al proyecto.

¬ŅPor qu√© la revisi√≥n (caracter√≠stica) es as√≠?


Hay una serie de preguntas √ļtiles para hacer.

¬ŅPor qu√© hacer una funci√≥n? ¬ŅPor qu√© se necesita esta caracter√≠stica? La respuesta a esta pregunta es importante.

¬ŅD√≥nde est√° el inicio del trabajo? En mi pr√°ctica, sucedi√≥ que me pidieron que volviera a hacer una determinada aplicaci√≥n. Hay una aplicaci√≥n A, necesita hacer una aplicaci√≥n B, que hace casi lo mismo con cambios menores. Empiezo a hacer el trabajo y resulta que A b√°sicamente no funciona. De hecho, se usa en alg√ļn lugar de la producci√≥n usando la interfaz hombre-m√°quina, es decir, alguien se sienta y reinicia constantemente el programa, arreglando la excepci√≥n de puntero nulo literalmente en el aire. ¬ŅD√≥nde est√° el inicio del trabajo? En este caso, ser√° en la fijaci√≥n del programa A para que funcione de manera estable, luego en la escritura del programa B.

¬ŅD√≥nde est√° la finalizaci√≥n completa del trabajo? ¬ŅC√≥mo debe ser ideal el trabajo realizado? Es importante formular desde el principio a d√≥nde va.

¬ŅD√≥nde est√° el final de la etapa actual? Est√° claro que no puedes comer un elefante de inmediato y es mejor dividir el trabajo en etapas. Es importante entender d√≥nde est√° el final de la etapa actual. A menudo, los proyectos se inflan y no terminan solo porque el alcance del proyecto se vuelve infinitamente grande.

¬ŅPor qu√© la funci√≥n se desglosa precisamente en tales etapas? Esto es sobre MVP y todo eso. Por favor piensa en esto tambi√©n.

Ahora sobre la API p√ļblica


Hay muchos art√≠culos sobre las propiedades de la API p√ļblica. L√©alo antes de implementarlo. Como buen ejemplo, puedo citar el framework JQuery, Spring en Java. Tambi√©n hay ejemplos negativos. Quien haya estado programando en Java durante a√Īos, probablemente recuerda lo que es terrible en t√©rminos de Public API EJB 2.1. La funcionalidad all√≠ puede ser buena, pero si la API p√ļblica es mala, nadie puede convencer a los usuarios para que usen el producto.

La API p√ļblica no es solo una herramienta para usuarios de terceros. Esto y la API de componentes internos, que usted mismo reutiliza entre ellos. Propiedades importantes de la API p√ļblica:

  • Simplicidad
  • La evidencia
  • Los valores predeterminados correctos. Vale la pena pensar en ellos si, por ejemplo, en un entorno de prueba crea 500 hilos, como en la producci√≥n. O viceversa, en la producci√≥n por defecto hay 3 hilos, como en un entorno de prueba.
  • Borrar mensajes de error. Este es el flagelo de tantos productos. Cuando algo cae dentro del sistema, no est√° claro qu√© se hizo mal. Ayer funcion√≥, hoy una excepci√≥n de puntero nulo. Lo que exactamente hizo mal y c√≥mo solucionarlo no est√° claro en el mensaje de error.
  • Es dif√≠cil cometer un error. Hay muchas recomendaciones sobre este puntaje. Las comprobaciones en tiempo de compilaci√≥n siempre son mejores que las comprobaciones en tiempo de ejecuci√≥n, etc.
  • Registros claros y suficientes. Esto es especialmente importante cuando escribe c√≥digo que se reutilizar√° y desplegar√° en alg√ļn lugar del servidor.
  • La capacidad de monitorear. Debe comprender que los registros y la supervisi√≥n tambi√©n forman parte de su API p√ļblica. Al analizar los errores, los usuarios ver√°n c√≥mo se comportan las m√©tricas que escupe en la supervisi√≥n.

Cambios de subsistema


Cuando se trata de la revisi√≥n del c√≥digo, es importante tener en su cabeza una lista completa de los sistemas y subsistemas de un producto grande en el que est√° cambiando. En proyectos empresariales, puede que no est√© claro: si se trata de un esquema de base de datos, o un controlador, o una presentaci√≥n, alg√ļn tipo de sistema de informes, cargas, descargas, etc.

Al trabajar con un producto en caja, es importante hacerse la pregunta: ¬Ņc√≥mo afectan los cambios a los procesos existentes en el sistema? ¬ŅHay una compatibilidad con versiones anteriores? ¬ŅAfecta el rendimiento? Si afecta, entonces el rendimiento de qu√©? ¬ŅC√≥mo afecta esto a la experiencia del usuario?

Procesos est√°ndar del sistema


Cada sistema empresarial tiene procesos est√°ndar: inicio, instalaci√≥n, alguna lista de operaciones. ¬ŅC√≥mo van a fluir ahora? Antes de revisar el c√≥digo, es importante entender esto. Debe pasar por el c√≥digo que implementa estos procesos. En el caso de Ignite, esto es:

  • Inicio / detenci√≥n de nodos (servidor / cliente, coordinador / regular): iniciar, detener un nodo, iniciar un servidor o nodo de cliente, iniciar un coordinador o un nodo normal
  • Uni√≥n de nodos: en t√©rminos de un nuevo nodo / coordinador / servidor / cliente
  • Activaci√≥n de cl√ļster (y desactivaci√≥n)
  • Cambio de coordinador
  • Crear / Eliminar cach√©
  • Persistencia / BLT / MVCC

Está claro que el conjunto de estos procesos es bastante amplio. Es importante comprender que tales procesos existen y cómo cambian.

Casos de esquina


En su aplicación empresarial, puede ocurrir recuperación ante desastres, inicialización inicial del sistema, apagado de nodos, reinicio, actualización de su aplicación, etc. En el caso de Ignite, tenemos los siguientes casos de esquina:

  • cambio de coordinador;
  • ca√≠da de nodo;
  • problemas de red: cuando los mensajes de red no llegan;
  • archivos rotos

Debemos verificar y verificar estas cosas para que todo esté en orden.

Buenas características de código


Entonces llegamos al código. Te aconsejo que no seas flojo y busques en él:

  • simplicidad
  • extensibilidad
  • comprobabilidad
  • fiabilidad
  • Alta velocidad de trabajo.

Concurrencia


Java tiene sus propias peculiaridades al escribir código de concurrencia. Si tiene un sistema de negocios y hay poca concurrencia allí, no necesita considerar estas características. Sin embargo, por lo general, algunas sincronizaciones pasan por la base de datos. En cosas como Ignite, esto es un poco más complicado. Y aquí, no solo la funcionalidad del código es importante, sino también sus propiedades:

  • ¬ŅQu√© tan dif√≠cil es entender su modelo de concurrencia?
  • Estructura de datos compartidos: ¬Ņc√≥mo se garantiza su integridad?
  • Cerraduras: ¬Ņqu√© y por qu√©?
  • Hilos: ¬Ņqu√© agrupaciones se utilizan? Por qu√©
  • Garant√≠as de visibilidad de los cambios: ¬Ņpara qu√© se proporcionan?

Estas preguntas deben hacerse antes de revisar el código de concurrencia. Está claro que esta lista puede continuar durante mucho tiempo.

Pruebas de desempe√Īo. Puntos de referencia


Si est√° desarrollando alg√ļn tipo de sistema, tiene clientes, instalaciones, obviamente funciona con cierto rendimiento. En el mundo de hoy, es imposible aumentar la potencia del hardware de forma indefinida. Necesitamos pruebas y monitoreo del desempe√Īo. Al realizar una revisi√≥n del c√≥digo, es importante comprender:

  • ¬ŅSon necesarias las pruebas de rendimiento? ¬ŅQuiz√°s este es alg√ļn tipo de refinamiento que no necesita pruebas de rendimiento?
  • Si es necesario, ¬Ņcu√°l? Existen muchas t√©cnicas y m√©todos de medici√≥n, etc.
  • ¬ŅD√≥nde, c√≥mo, qu√© se debe medir?
  • ¬ŅQu√© puntos de referencia son indicativos? ¬ŅEl n√ļmero de nodos? Hierro? ¬ŅEncender configuraci√≥n? La naturaleza de la carga?

Total


En general, la revisión de código es una práctica muy gratificante. Espero que todos los desarrolladores (incluidos los productos empresariales) ya lo hayan implementado en casa. De lo contrario, implemente lo antes posible. Estaré encantado de discutir las prácticas de revisión de código con usted en los comentarios.

Video de la conferencia:

La presentación está disponible aquí .

Source: https://habr.com/ru/post/445440/


All Articles