RafaelT00 / AutoIV

Prácticas IV
GNU General Public License v3.0
0 stars 0 forks source link

[IV-22-23] Objetivo 2 #13

Closed ignaciotitos closed 1 year ago

ignaciotitos commented 1 year ago
ignaciotitos commented 1 year ago

Necesito que asignes los issues al Milestone 1 y este PR al Milestone 0

ignaciotitos commented 1 year ago

También mira a ver que te parece el código como boceto. Si añadirías algo o no.

RafaelT00 commented 1 year ago

Necesito que asignes los issues al Milestone 1 y este PR al Milestone 0

Listo.

También mira a ver que te parece el código como boceto. Si añadirías algo o no.

Creo que con eso está bien. Al menos para lo que tengo en mente debería bastar.

ignaciotitos commented 1 year ago

Perfecto. Pues le daré un repaso para marcar todos los puntos y si estás de acuerdo el lunes/martes lo enviamos para que lo corrija JJ.

RafaelT00 commented 1 year ago

Perfecto.

ignaciotitos commented 1 year ago

Perdón, necesito que los issues se asignen al milestone 0, perdón por la equivocación.

RafaelT00 commented 1 year ago

Me parecia raro a mi también, jajaja, bueno ya está listo.

ignaciotitos commented 1 year ago

Yo creo que ya estaría. Me ha faltado poner los commits con el formato estándar y de buenas prácticas. Si sabes alguna manera de cambiarlo, lo haré.

RafaelT00 commented 1 year ago

Creo que puedo solicitar cambios en los commits. Hay dos que simplemente ha sido crear y un directorio y luego borralo no?. Supongo que esos dan igual.

ignaciotitos commented 1 year ago

Sí. Para añadir la carpeta en los que contiene los ficheros más la creación del fichero iv.yaml que lo cree sin querer para otro pull request.

ignaciotitos commented 1 year ago

Yo creo que ya estaría para revisión. Tu me dices si ves que hace falta cambiar algo o lo que sea.

ignaciotitos commented 1 year ago

Cuando lo apruebes mencionamos a JJ.

RafaelT00 commented 1 year ago

He solicitado cambios sin querer.

ignaciotitos commented 1 year ago

Perfecto.

danielsp13 commented 1 year ago

Buenas tardes chicos, he estado haciendo una revisión sobre lo que habéis estado haciendo y tengo que comentaros varios asuntos.

Lo primero de todo, es que ningún issue de los planteados está relacionado con alguna de las Historias de Usuario que hay en este proyecto.

En consecuencia de esto, el código que está actualmente implementado no es válido, ya que no se puede verificar si está bien o no.

danielsp13 commented 1 year ago

Viendo que @RafaelT00 sólo tiene una historia de usuario: #7 . @ignaciotitos , te recomiendo que lo analices y a partir de ahí extraigas lo que se necesita para este objetivo y para satisfacer dicha historia.

Los issues deben plantear problemas que se relacionen con dicha HU, más allá de necesito tal atributo o tengo que hacer x clase. La solución que plantees para resolver dicho issue, debe ir en el código.

Es necesario que sigas este esquema de HU -> Issue -> Solución. Si no, no se te puede dar por bueno este objetivo @ignaciotitos .

danielsp13 commented 1 year ago

También, se necesita documentar en un fichero el análisis que has realizado, y especifiques por qué has decidido implementar una cosa u otra.

danielsp13 commented 1 year ago

Finalmente, commits como https://github.com/RafaelT00/AutoIV/pull/13/commits/295218a8bb275099a40c1f8e583c9ab4e9cd40a8 no siguen las mejores prácticas. Te recomiendo que también le eches un vistazo a ese asunto.

ignaciotitos commented 1 year ago

Okey @danielsp13, gracias por tu ayuda. Mañana lo cambiaré. En respecto a lo del commit, sí es verdad que no sigue las mejores prácticas, lo hice sin querer y me di cuenta luego, pero no se como cambiarlo. ¿Tu sabes como?

danielsp13 commented 1 year ago

Okey @danielsp13, gracias por tu ayuda. Mañana lo cambiaré. En respecto a lo del commit, sí es verdad que no sigue las mejores prácticas, lo hice sin querer y me di cuenta luego, pero no se como cambiarlo. ¿Tu sabes como?

No te preocupes. A mi también me ha pasado cuando empecé. No es necesario que cambies los commits que tienes ahora mismo. Sólo sigue a partir de ahora con las buenas prácticas y eso será suficiente 😀

danielsp13 commented 1 year ago

Te lo digo porque manera sí hay... Pero es una mala práctica en general. Así que, no es necesario que lo hagas por ahora siempre y cuando el resto sí que sean correctos.

ignaciotitos commented 1 year ago

Hola @danielsp13, he cambiado algunas cosas, por lo que si puedes echarle un vistazo cuando puedas, gracias!

danielsp13 commented 1 year ago

Buenas tardes @ignaciotitos , estoy revisando todo, ahora te comento .

ignaciotitos commented 1 year ago

Hola @danielsp13, creo que ya está un poco mejor. Cuando puedas revísalo y con lo que sea me dices. Gracias!

JJ commented 1 year ago

Por favor, sigue lo comentado por @danielsp13 y lo que hemos hablado en clase.

ignaciotitos commented 1 year ago

@RafaelT00 aprueba los cambios cuando puedas o sugiere cambios en cualquier cosa.

danielsp13 commented 1 year ago

Hola @ignaciotitos , por alguna razón que desconozco no he visto nuevos cambios ni menciones hasta ahora. Voy a echarle un vistazo.

danielsp13 commented 1 year ago

En la documentación src/DOC.md:

Para esto, el primer issue necesario ha sido la https://github.com/RafaelT00/AutoIV/issues/8, en el cual se han propuesto varios tales como Python, Ruby, C++, pero al final el elegido ha resultado ser Go, ya que el compañero quería aprenderlo. Después de asignar el lenguaje me surgió la duda de las https://github.com/RafaelT00/AutoIV/issues/9, que están en el Milestone 0 y se pueden sacar algunas de la HU, pero quería quitarme la duda. Para esto se ha sacado que se necesita de la clase Empleado, Vacaciones y Restricciones.

No me parece un párrafo relevante para lo que se requiere en este aspecto, para eso está el fichero iv.yaml. Salvo que haya un motivo necesariamente importante y que no esté justificado en el correspondiente issue, no lo pondría.


Para cada clase ha sido necesario saber cómo va a funcionar cada clase, para saber si van a ser inmutables o mutables. Para esto ha sido necesario hablar con Rafael y entender cómo se van a asignar las distintas clases. Por ello, las clases Restricciones y Vacaciones, van a ser entidades ya que van a ser mutables, mientras que Empleado que iba a ser inmutable de primeras, con los nuevos cambios que se hicieron añadiéndole variables que si van a cambiar durante el ciclo de vida de esta clase cuando se instancie, va a ser otra entidad. En este caso no hay objetos valor, no hay clases inmutables en el tiempo.

Así con Empleado se soluciona el hecho de crear empleados, ya que es necesario estos para asignar y que les asignen vacaciones. Con la clase Vacaciones se tiene un calendario de los días en los que se puede asignar vacaciones a los empleados, y con la clase Restricciones se soluciona el hecho de que no les puedes asignar vacaciones a los empleados a la vez, tienes que llevar un conteo y poder saber cuando asignar a un empleado o a otro.

Supongo que tendrás que darle una vuelta tras todos estos cambios que has ido realizando.

ignaciotitos commented 1 year ago

Vale. Cambiado. Gracias por la ayuda @danielsp13.

ignaciotitos commented 1 year ago

@RafaelT00 cuando puedas aprueba o sugiere cambios.

ignaciotitos commented 1 year ago

Tienes que aprobar los cambios @RafaelT00

ignaciotitos commented 1 year ago

Listo para revisión @JJ

JJ commented 1 year ago

Ni un solo commit tiene el issue al que se refiere en primera línea. Eso aumenta el trabajo que tengo que hacer, pulsando en cada uno para ver si está en el resto del cuerpo. Os agradecería que siguiérais las buenas prácticas repetidas en clase y en los errores frecuentes, porque todas tienen su razón de ser.

JJ commented 1 year ago

Necesito que asignes los issues al Milestone 1 y este PR al Milestone 0

Listo.

También mira a ver que te parece el código como boceto. Si añadirías algo o no.

Creo que con eso está bien. Al menos para lo que tengo en mente debería bastar.

Es casi imposible que esté bien; siempre habrá alguna cosa que no corresponda a las historias de usuario o que no encaje del todo. Te recuerdo que no se trata de que @ignaciotitos averigüe lo que tienes en mente, sino que los dos tratéis de modelizar correctamente, siguiendo en lo posible domain driven design el problema. Si no, es dejarme la labor de evaluar a mi (lo tengo que hacer de todas formas y lo hago con gusto, pero en la revisión y corrección los dos aprenderéis algo)

ignaciotitos commented 1 year ago

Ni un solo commit tiene el issue al que se refiere en primera línea. Eso aumenta el trabajo que tengo que hacer, pulsando en cada uno para ver si está en el resto del cuerpo. Os agradecería que siguiérais las buenas prácticas repetidas en clase y en los errores frecuentes, porque todas tienen su razón de ser.

Busqué por internet en varios sitios y ponían el issue al final del todo.

JJ commented 1 year ago

Ni un solo commit tiene el issue al que se refiere en primera línea. Eso aumenta el trabajo que tengo que hacer, pulsando en cada uno para ver si está en el resto del cuerpo. Os agradecería que siguiérais las buenas prácticas repetidas en clase y en los errores frecuentes, porque todas tienen su razón de ser.

Busqué por internet en varios sitios y ponían el issue al final del todo.

¿Entiendes por qué hace falta que el issue esté en la primera línea, y por qué lo he dicho así repetidamente en clase, y repetido también en la sección de "errores frecuentes" de alguna semana, y en las presentaciones que he hecho? Se trata de que entendáis por qué hacéis las cosas. "Buscar por internet" no es justificación suficiente (ninguna justificación, de hecho) para no seguir las mejores prácticas, cuando además se te ha dado la razón por la que se ha pide que se haga así.

ignaciotitos commented 1 year ago

Ya cambié el path del documento DOC.md desde Git Bash y le cambié el nombre al fichero.

ignaciotitos commented 1 year ago

Ni un solo commit tiene el issue al que se refiere en primera línea. Eso aumenta el trabajo que tengo que hacer, pulsando en cada uno para ver si está en el resto del cuerpo. Os agradecería que siguiérais las buenas prácticas repetidas en clase y en los errores frecuentes, porque todas tienen su razón de ser.

Busqué por internet en varios sitios y ponían el issue al final del todo.

¿Entiendes por qué hace falta que el issue esté en la primera línea, y por qué lo he dicho así repetidamente en clase, y repetido también en la sección de "errores frecuentes" de alguna semana, y en las presentaciones que he hecho? Se trata de que entendáis por qué hacéis las cosas. "Buscar por internet" no es justificación suficiente (ninguna justificación, de hecho) para no seguir las mejores prácticas, cuando además se te ha dado la razón por la que se ha pide que se haga así.

A partir de ahora lo haré así.

ignaciotitos commented 1 year ago

Partiendo de esto: Captura de pantalla de 2022-12-13 14-21-35 el resto no puede estar bien. A estas alturas de la asignatura no puedes usar la web para desarrollar, sino la línea de órdenes o el IDE. Debes aprender a usar las herramientas más adecuadas para el desarrollo. Por no mencionar que los mensajes de commit deben seguir las buenas prácticas. Por favor, revisa los objetivos de aprendizaje de las últimas semanas, repasa si de verdad estás haciendo lo que dice en la lista de comprobación, y los errores frecuentes de este objetivo.

Entiendo. Me pareció en el momento la forma más fácil y rápida de hacerlo. A partir de ahora lo haré con Git Bash que ya he aprendido y me puedo defender un poco.

ignaciotitos commented 1 year ago

@JJ para revisar otra vez.

ignaciotitos commented 1 year ago

He hecho algunos cambios en los que creo que ha podido quedar un poco más claro. @RafaelT00 @JJ

ignaciotitos commented 1 year ago

Listo para revisión. @JJ

ignaciotitos commented 1 year ago

@RafaelT00 solicito revisión.

ignaciotitos commented 1 year ago

Pero entonces como va a asignar las vacaciones? Ya que habría que diferenciar entre un empleado y el encargado que asigna las vacaciones.

RafaelT00 commented 1 year ago

Es el software el que asigna las vacaciones a los empleados. Los encargados son los que lo usarían, quiero decir, no se si tiene sentido representar a quien usa el software.

ignaciotitos commented 1 year ago

Vale, entiendo. Cambio de Encargado a Empleado

ignaciotitos commented 1 year ago

Ya estaría. Si te parece bien aprueba los cambios y menciono al profesor.

ignaciotitos commented 1 year ago

Listo para revisión @JJ

ignaciotitos commented 1 year ago

Para empezar, los issues tienen que estar asignados a un milestone. Hay que centrarse, también, en un milestone determinado, y hacer un producto mínimamente viable. Esto debe ser un modelo coherente de al menos parte del dominio del problema, partiendo de las historias de usuario. También conviene seguir buenas prácticas mínimas sobre nombres de variables.

Fallo mío, no me acordé se decírselo al compañero. Ya está resuelto.

ignaciotitos commented 1 year ago

Listo para revisión @JJ