Reviewing a Patch/es
Como colaborar con PostgreSQL: Revisión de Parches
Muchos piensan que no tienen los conocimientos necesarios como para hacer una revisión completa de los parches enviados a PostgreSQL. Si todavía no sos committer, técnicamente esa afirmación es cierta, al menos en ese sentido, y debido a que el proyecto es tan extenso, resulta bastante difícil comprender todo el código base. Pero la revisión incluye diferentes tareas, y aunque no puedas hacerlas todas, podés colaborar con el proyecto en las primeras etapas.
Si podés aplicar un parche y usar la nueva característica, ya podés empezar a revisarlo!
El committer correspondiente hará la revisión necesaria antes de ingresar el parche en el código base. La tarea de quien realiza la revisión es descomprimir la carga de trabajo de los committers encargándose de los problemas simples. El trabajo de quien realiza la revisión no consiste en garantizar el nivel de calidad sino solamente informar cualquier tipo de problema que identifiquen. La tarea está lista si pensás que el parche ya está listo para ser sometido a una revisión más rigurosa por parte de un committer. Ver esta revisión de parche como ejemplo del resultado obtenido tras una revisión más rigurosa. Las revisiones para otro parche pueden contener, por cierto, diferentes secciones e incluso tener una apariencia completamente diferente.
Si podés hacer cualquiera de las siguientes tareas, tu ayuda va a ser bienvenida. Ver también la presentación de Josh Tolley para ver una descripción irónica, pero no por eso menos precisa, de quienes pueden participar.
Posibles etapas de una revisión de parche:
Índice
- Revisión de la entrega (conocimientos necesarios: implementar el parche, comprensión del idioma inglés)
- Revisión de usabilidad (conocimientos necesarios: test-fu, poder encontrar y leer el archivo spec)
- Prueba (conocimientos necesarios: implementar el parche, configurarlo, hacerlo, registrar los errores)
- Revisión de comportamiento (conocimientos necesarios: capacidad de calcular el comportamiento)
- Revisión de la programación (conocimientos necesarios: comparar lineamientos, experiencia con problemas de portabilidad, conocimientos básicos de lectura de código C)
- Revisión de arquitectura (conocimientos necesarios: experiencia en toda la arquitectura del proyecto PostgreSQL)
- Revisión de la revisión
Revisión de la entrega (conocimientos necesarios: implementar el parche, comprensión del idioma inglés)
- Verificar si el parche está context diff format|en formato diff
- ¿Se aplica sin problemas en el actual master de git?
- ¿Incluye pruebas razonables, los parches doc necesarios, etc.?
Revisión de usabilidad (conocimientos necesarios: test-fu, poder encontrar y leer el archivo spec)
Leer lo que se supone que debe hacer el parche, y evaluar:
- ¿El parche lo implementa en efecto?
- ¿Es lo que queremos que pase?
- ¿Es todo lo que necesitamos?
- ¿Sigue las especificaciones de SQL o lo que se acostumbra hacer en la comunidad?
- ¿Incluye soporte para pg_dump (de corresponder)?
- ¿Hay algún peligro relacionado?
- ¿Se han cubierto todas las bases?
Prueba (conocimientos necesarios: implementar el parche, configurarlo, hacerlo, registrar los errores)
Aplicar el parche, compilarlo y probar:
- Si funciona según lo anticipado
- ¿Hay casos fuera de los parámetros normales que el autor no tuvo en cuenta?
- ¿Hay fallas o caídas?
- Las revisiones se deberán realizar con las opciones configure , --enable-cassert y --enable-debug habilitadas; |Trabajar en CVS para ver un ejemplo completo. De esa forma, se podrán encontrar errores en el código que de otra manera se pasarían por alto. Tené en cuenta que una copia de PostgreSQL creada utilizando estos parámetros será mucho más lenta que la versión creada sin ellos. Si estás trabajando en algún tema relacionado con el comportamiento, como por ejemplo probar si un parche baja la velocidad de algún otro componente, asegurate de crearlo sin esos parámetros antes de probar la velocidad de ejecución. Algunas, pero no todas, las penalidades de --enable-cassert se pueden deshabilitar al iniciar el servidor ingresando debug_assertions = false en tu postgresql.conf. Ver Opciones para Desarrolladores para ver más detalles acerca de esa configuración; vuelve al valor inicial true al utilizar --enable-cassert. Tené en cuenta también que mientras que --enable-debug no debería tener penalidades al trabajar con gcc, definitivamente las tiene con la mayoría de los demás compiladores.
Revisión de comportamiento (conocimientos necesarios: capacidad de calcular el comportamiento)
- Verificar si el parche desacelera las pruebas simples
- Si dice mejorar el rendimiento, ¿lo hace?
- ¿Desacelera algún otro proceso?
Revisión de la programación (conocimientos necesarios: comparar lineamientos, experiencia con problemas de portabilidad, conocimientos básicos de lectura de código C)
Leer los cambios realizados en el código detalladamente y evaluar:
- ¿Cumple con los lineamientos de programación del proyecto?
- ¿Hay problemas de portabilidad?
- ¿Funcionará en Windows/BSD, etc.?
- Los comentarios ¿son los suficientes y precisos?
- ¿Hace lo que dice, y correctamente?
- ¿Produce advertencias de compilador?
- ¿Podés hacer que caiga?å
Revisión de arquitectura (conocimientos necesarios: experiencia en toda la arquitectura del proyecto PostgreSQL)
Teniendo en cuenta los cambios realizados en el código en el contexto del proyecto en general:
- ¿Se hace todo de manera de manera coherente con otras características/módulos?
- ¿Hay interdependencias que puedan ocasionar problemas?
Revisión de la revisión
- Quien realizó la revisión, ¿se encargó de revisar todo lo que ese tipo de revisión debe incluir?
