Encontré un documento con reglas y recomendaciones sobre el proceso de revisión de código dentro de la empresa. Decidí que esa información útil debería compartirse con el mundo exterior. Con la bendición del
autor, publico el trabajo.
La mayoría de estas reglas son de naturaleza consultiva y deben guiarse principalmente por el sentido común y no por una observación ciega.
Disposiciones generales
- En hh.ru, la revisión se realiza en el formato de solicitud de extracción en Github.
- Se requieren solicitudes de revisión y extracción, incluso si dentro del marco de la tarea se cambia una línea o un carácter en el código.
- Cada tarea debe tener al menos un revisor responsable, se indica en la tarea como revisor y es responsable del código, como el autor. No está prohibido y alentado a participar en la revisión de otras personas.
- La responsabilidad de realizar la tarea a través de la revisión recae en el autor de la tarea.
- Cualquier cambio después de una revisión, como las revisiones durante la prueba, también debe ser visto previamente.
Revisar objetivos
- Difusión de experiencia y conocimiento entre desarrolladores.
- Apoyo al principio: los cambios no deberían empeorar el código existente, deberían mejorarlo.
- Corrección de errores en la lógica y errores relacionados con la seguridad, manejo correcto de excepciones.
- Mejora de la calidad del código, la mantenibilidad y la arquitectura del proyecto.
- Mejora de la legibilidad del código.
- Mejora de la cobertura de pruebas y pruebas en el nivel correcto.
- Mejora de la documentación.
- Cumplimiento de los cambios con los requisitos de la tarea.
- Prevención de deuda técnica o impuesto técnico.
- El diseño atento de la API, donde la API es cualquier módulo con una interfaz externa. Por ejemplo, que un recurso en un servicio tiene una URL bien pensada, un formato JSON conveniente, códigos de respuesta correctos en diferentes casos, etc.
- El historial correcto de confirmaciones en el Gita, con mensajes que reflejan la esencia del mensaje, sin confirmaciones temporales.
Preparación para la revisión
- Antes de publicar una solicitud de extracción, lea su código 2-3 veces: con una alta probabilidad, usted mismo encontrará errores allí, lo que le ahorrará tiempo al revisor. Compruebe que no haya errores que puedan detectarse automáticamente: errores de estilo de código, pruebas descartadas. Asegúrese de que el código no tenga comentarios temporales y código de depuración, y también verifique que su código coincida con las guías de estilo aceptadas.
- Si crea una solicitud de extracción en el proceso de trabajar en una tarea, escriba "[WIP]" en el encabezado y coloque una marca de "trabajo en progreso". Cuando finalice el trabajo, elimine las etiquetas e informe al respecto en un comentario por separado para que las partes interesadas sepan que se puede ver el código.
- Esté preparado para mostrarle al revisor una mini demostración de la tarea.
- Dé pequeñas porciones de código para su revisión. 200-300 líneas de cambio: el límite para una revisión efectiva.
- Realice cambios independientes en confirmaciones separadas.
- Describa los compromisos con mensajes cortos e informativos.
- Refactorizando como una confirmación por separado, es más fácil ver los cambios en la lógica, la esencia de la tarea.
- Formatee el código como una confirmación por separado antes de cambios importantes, o incluso como una solicitud de extracción por separado.
- No cometer cambios menores. Por ejemplo, agregar saltos de línea en un archivo en el que no cambió nada más.
- En el título de la solicitud de extracción, describa breve e informativamente la esencia de los cambios.
- Describa el problema y la solución en la solicitud de extracción. Describa soluciones alternativas y por qué se selecciona la solución actual. Si los cambios se relacionan con la interfaz, adjunte capturas de pantalla "antes" y "después".
- En la solicitud de extracción, proporcione un enlace a la tarea y en la tarea un vínculo a la solicitud de extracción.
- A veces es útil discutir la solución elegida con el revisor antes de escribir el código. Esto ayudará a encontrar la mejor solución al problema y simplificará el proceso de revisión.
Elección del revisor
- Dele la tarea de la revisión a un especialista en el campo relevante.
- Si varios comandos funcionan con algún código, es útil seleccionar un revisor de otro equipo para difundir el conocimiento.
- El oficial de libertad condicional puede no ser el revisor responsable, pero puede participar en el proceso de revisión.
- No asigne todas sus tareas a la revisión a las mismas personas; solicite revisiones de diferentes personas.
- Si el problema involucra un código no trivial que cierta persona entiende, muéstrele los cambios, independientemente de quién sea el revisor responsable. También recuerde que cada repositorio tiene mantenedores, ellos saben más sobre este código y supervisan el desarrollo.
- El resto del revisor se elige arbitrariamente, por mutuo acuerdo de las partes. Use las recomendaciones en el github basadas en la "culpa" del código afectado para ayudarlo a elegir.
- Después de seleccionar un revisor, especifíquelo como Asignado en la solicitud de extracción. Una lista de todas las solicitudes de extracción que se le asignan.
Proceso de revisión, general
Se refiere tanto al autor del código como al revisor.
- Todo el código es común, no existen conceptos como "mi código" o "su código". Esto significa que cada desarrollador es responsable de cualquier línea de código, y no solo el que escribió esta línea.
- Cree una atmósfera de entendimiento mutuo, sea cortés y amable, valore y respete a los demás, no imponga su opinión. Escuche la opinión del oponente y no defienda su principio. No convierta la revisión en una experiencia negativa para los colegas.
- Haga preguntas si algo no está claro. Argumente y especifique preguntas y respuestas. Puede que no sea obvio para el autor del código lo que se entiende por la pregunta "¿por qué es así?", Pero el revisor no entiende la argumentación de la respuesta "en desacuerdo".
- No alargue el proceso de revisión, ahorre tiempo, responda rápidamente a los comentarios, discuta verbalmente, no provoque largas discusiones y no genere "holivars". Si no puede llegar rápidamente a un consenso, busque la ayuda de sus colegas, el responsable o el gerente del área funcional.
- Si la tarea no está en un par de líneas, pase 10-15 minutos con el revisor en una revisión conjunta del código, es útil hacerlo incluso antes de crear una solicitud de extracción. Discuta la esencia de la tarea y la solución, vea cómo fue y cómo se convirtió en un entorno de trabajo. Esto ayudará al revisor a sumergirse mejor en el contexto de la tarea. La mayoría de los comentarios permanecerán sin escribir, lo que ahorrará tiempo a todos.
- Después de una discusión oral, siempre describa la decisión tomada y las razones para ello en la solicitud de extracción. La responsabilidad de la descripción recae en el autor del código.
- Si el código contiene el mismo tipo de errores, el revisor debe centrarse en la atención del autor del código en el primer o segundo comentario sin comentar cada entrada, y el autor debe analizar el código para el mismo tipo de errores antes de publicar la corrección.
- Elogie las decisiones exitosas del autor y las sugerencias del revisor.
- Deje comentarios sobre los cambios en la solicitud de extracción en sí, y no sobre confirmaciones individuales. Por lo tanto, toda la correspondencia estará en un solo lugar, incluso después del rebase.
- Responda a los comentarios en los mismos hilos de discusión donde comenzaron. Si el comentario se refiere a la solicitud de extracción completa o varios comentarios en el mismo hilo, cite el comentario comentado. Para cambiar de un correo electrónico a un hilo de discusión desactualizado, en lugar del enlace "verlo en GitHub", use el enlace "en ruta / a / archivo".
- Si durante el proceso de revisión se toman decisiones fundamentales desde el punto de vista de los estándares de codificación u otros acuerdos formalizados, el autor del código está obligado a anotar estas decisiones en los documentos relevantes.
- La revisión no es solo sobre el código, sino sobre toda la tarea. No trate los requisitos de la tarea o los diseños como la verdad definitiva: todos cometen errores y nadie puede tener en cuenta todos los matices. Una apariencia fresca y sugerencias sensatas solo beneficiarán al producto.
Proceso de revisión por autor del código
- La tarea no se completó hasta que pasó la revisión, prueba y lanzamiento en el "producto".
- Responda a todos los comentarios del revisor, no deje comentarios sin responder, independientemente de si los aceptó o no.
- Piense en las preguntas que surgen o pueden surgir durante la revisión. Quizás a la sección de código le falta un comentario o es mejor escribir el código más explícitamente.
- No corrija los commits existentes con un commit gend --amend; en cambio, realice los cambios como commits separados, uno para cada arreglo. La corrección de las confirmaciones existentes complica en gran medida el proceso de revisión, ya que debe revisar todo el código nuevamente.
- Después de agregar un nuevo compromiso a la solicitud de extracción, escriba en un comentario separado un resumen de los cambios para que las partes interesadas estén al tanto. Esto se aplica a las correcciones durante la revisión, así como a las correcciones durante las pruebas.
- Después de la revisión, antes de enviar la tarea para la prueba, vuelva a escribir el historial de confirmaciones (git rebase --interactive), haciendo correcciones a las confirmaciones individuales y eliminando las confirmaciones temporales. Consulte la opción --autosquash para simplificar el proceso.
Proceso de revisión por revisor
- Si está ocupado en el momento de la solicitud de revisión, avíseme exactamente cuándo comenzará la revisión.
- No exija, pero sugiera hacer cambios.
- Comente el código, no la persona que lo escribió.
- Usted asume la responsabilidad del código escrito, aborda la revisión en consecuencia, pero esto no significa que el autor del código deba cumplir estrictamente con todos sus requisitos. Recuerde, se pueden hacer muchas cosas de diferentes maneras.
- Siga los objetivos de la revisión y sugiera ediciones sustantivas. No insista en cambios no críticos. Indique la importancia de las correcciones en un comentario.
- Si encuentra un problema grave, pause la revisión y espere a que el autor la solucione. Lo más probable es que después de la corrección aún sea necesario revisar nuevamente.
- Preste atención no solo a lo que se ha cambiado, sino también a lo que no se ha cambiado. Busque y muestre al autor el código que debería haber cambiado, pero que no lo fue (importaciones olvidadas o clases no utilizadas, utilizando código refactorizado en otro lugar, etc.).
- Después de hacer cambios por el autor del código, revise las correcciones y revise toda la solicitud de extracción nuevamente. Quizás, sujeto a correcciones, tendrá nuevos comentarios o preguntas.
- No pierda el tiempo revisando el código que no ha cambiado como parte de la tarea. La asignación de una parte del código a una función se considera un cambio. La transferencia de código "como está" no se considera un cambio. La reforma del código en el archivo afectado queda a discreción del autor.
- El revisor no edita de forma independiente el código en la rama, porque lo quiere mucho o más fácil. Los cambios los realiza el autor del código.
- Si realizó una revisión, intente no retrasar o cambiar a otras tareas en detrimento de la actual.
Materiales usados y enlaces relacionados
PD Comparta sus reglas, recomendaciones y casos de usuarios útiles sobre el proceso de revisión en los comentarios.