Actividad 3 - GRUPO 4

[ruipablo]

https://github.com/MatyRusso22/vetsoft

[ivanduva]

Buenas chicos,
Hay muchos problemas en la app que deben corregir para aprobarla. El principal es que faltó hacer una migración, por lo cuál la app no anda ni pasan los tests a menos que la hagan. Además luego de mergear el PR sin la migración, siguieron mergeando PRs arrastrando el error. Esto puede indicar que no están probando la app al hacer la review o lo están haciendo mal, cuestión que ya les marcamos en correcciones anteriores.
También encontramos otros problemas que hay que corregir:

• En el caso de la dirección del provider, no hay tests específicos sobre la dirección sino que son generales sobre la clase Provider. Los tests están bien pero son más generales sobre la clase Provider. Cuando se soluciona un issue, se debería agregar tests relativos a la funcionalidad que describía ese issue.
• En el caso del peso de la mascota, el form permite guardar peso 0 y permite negativos que no guarda. Además no están haciendo el raise ValidationError en la validación del modelo. Por eso es que no tira el error en los tests. En el de integración el status 302 de la response no significa error, sino una redirección. Por el lado de los e2e directamente no hay tests específicos sobre el peso.
• En cuanto a la especialidad del veterinario, además del tema de la migración que falta, está hecho a medias el tema de la clase enumerativa. No aparecen las opciones en el form. Si lo van a hacer con la enumerativa, deberían mostrarse las opciones, si no aparecen la clase no tiene sentido. Además tanto en e2e como en integración no hay tests específicos sobre la especialidad.
• En cuanto al precio del producto, faltan tests específicos sobre ese atributo en e2e y en unidad.

Cada cosa debería solucionarla quien se encargó de la tarea.

Además vemos problemas de procedimiento que tienen que mejorar en las próximas entregas, tanto correcciones como actividades:

• En las reviews no hay comentarios con relevancia. Como dijimos más de una vez, comentarios del estilo "Funciona bien" no aportan a la review. Para eso se pone el approve y listo. Además había errores claros para marcar, por ejemplo el de la migración que aparece solo al correr la app. No es algo relativo al código que puede ser complicado de encontrar.
• No es la mayoría, pero encontramos algunos commits con mensajes malos. Por ejemplo: MatyRusso22/vetsoft@dc2ec62 . Como repetimos más de una vez, los mensajes deben contener una síntesis de la funcionalidad que el commit soluciona.

Por otro lado, valoramos que mejoraron bastante con el tema de los commits y la solución de conflictos. En ese sentido, sigan así.
Saludos!

[MatyRusso22]

Buenas profe, ya corregimos los bugs que nos marco: https://github.com/MatyRusso22/vetsoft

[ivanduva]

Buenas,
Queda aprobado, pero sigue habiendo muchos problemas de procedimiento. Vimos que mergearon PRs con test fallando y con problemas en las migraciones. La review en general no es buena. Seguimos sin ver comentarios relevantes, a veces relacionan el issue al PR y a veces no, entre otros problemas.
Además dejaron un funcionamiento extraño en el form de las mascotas. Cuando hay un error, borra todos los campos que ya completó el usuario. Lo mismo al editar, no aparecen algunos campos ya completados. Esto es importante no tanto por el código sino porque es raro que nadie lo haya mencionado en la review.
Si estas cuestiones no mejoran en la próxima actividad, es probable que sigan teniendo que corregir.

Saludos