Skip to content

[IV-23-24] Objetivo 2#5

Open
danibarranqueroo wants to merge 21 commits into
raultl12:mainfrom
danibarranqueroo:Objetivo-2
Open

[IV-23-24] Objetivo 2#5
danibarranqueroo wants to merge 21 commits into
raultl12:mainfrom
danibarranqueroo:Objetivo-2

Conversation

@danibarranqueroo

@danibarranqueroo danibarranqueroo commented Oct 19, 2023

Copy link
Copy Markdown

Objetivo2

Sobre la estructura del repositorio

  • ¿He seguido las mejores prácticas en el nombre de las clases y ficheros y
    disposición de los mismos?
  • ¿He mirado tanto reglas generales (para cualquier repo) como reglas
    específicas para el lenguaje con el que se está trabajando?

Sobre el planteamiento

  • ¿Se han planteado una serie de issues, asignados a las historias de
    usuario pertinentes, que clarifiquen qué es lo que necesitan los usuarios e
    identifiquen las diferentes partes del problema?
  • ¿Los issues representan un problema, y no una tarea?

Sobre el análisis del problema

  • ¿Se ha documentado qué análisis se ha hecho sobre el dominio para decir lo
    que se ha creado?
  • ¿Se ha documentado por qué se ha elegido que lo creado sea un objeto valor,
    una entidad o un agregado?
  • ¿He propuesto un producto mínimamente viable, que en muchos casos será un solo
    objeto valor que no dependa de ningún otro (y que sea la base de muchos
    otros)?.

Sobre el código

  • ¿Todos los mensajes de commit explican el cambio, y no se
    limitan a repetir el nombre del fichero que se ha cambiado?
  • ¿Los mensajes de commit siguen el formato estándar y buenas prácticas?
  • ¿Se ha hecho una revisión real del código para comprobar que todos
    los atributos y funciones creadas están respaldadas por una HU?
  • ¿Todos los issues creados están relacionados con a una HU?
  • ¿Ha asignado el propietario/la propietaria del repositorio todos los
    issues planteados al milestone correspondiente?
  • ¿Todos los cambios en el código están asignados a un issue al que se
    referencia en un commit?
  • ¿Se ha asignado al mismo milestone el PR que se ha hecho?
  • ¿Es el milestone sobre el que estoy trabajando un producto
    mínimamente viable? ¿O tengo que solicitar al product manager que precise de
    qué producto se trata?
  • ¿Se ha comprobado que el código entregado sea sintácticamente
    correcto?

@danibarranqueroo danibarranqueroo marked this pull request as draft October 19, 2023 11:26
@danibarranqueroo

Copy link
Copy Markdown
Author

Hola @raultl12 en este pr es donde iremos trabajando, cuando puedas enlázalo a tu milestone 0 y añade el tag de hacktoberfest-accepted.
Cualquier duda o problema que tengas sobre lo que voy haciendo puedes comentar sin problema!

@danibarranqueroo

Copy link
Copy Markdown
Author

Buenas tardes @raultl12! Cuando puedas revisa un poco todo lo que he ido haciendo y si algo no lo ves correcto o no te gusta la forma en la que lo he enfocado comenta sin problema tanto en los issues, como aquí o en los ficheros y vamos así perfeccionando el objetivo. Cuando lo veas listo acuérdate de poner por aquí la lista de comprobación 🙌.

@danibarranqueroo danibarranqueroo marked this pull request as ready for review November 20, 2023 07:27
Comment thread src/Habilidad.ts Outdated
Comment thread src/Habilidad.ts Outdated
@danibarranqueroo

Copy link
Copy Markdown
Author

Buenos días @raultl12 ya he reaizado los cambios propuestos, si no hay ningún otro problema o cosa que piensas que podríamos hacer de otra forma confírmame y procedo a poner este pr para revisión.

@raultl12 raultl12 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hola @danibarranqueroo. Lo veo bien para pedir revisión

@raultl12

raultl12 commented Nov 21, 2023

Copy link
Copy Markdown
Owner
  • ¿Los issues que ha planteado la persona son efectivamente problemas y
    evidencian comprensión de las historias de usuario? Si no es así, es mejor que
    se le transmita antes de que empiece a crear cualquier código erróneo.
  • ¿Has comprobado que efectivamente el compañero/a ha seguido su lista de
    comprobación?
  • ¿El código creado es una modelización válida de las historias de usuario
    creadas por ti, y te sirve como base para programar la lógica de negocio?
  • ¿El código creado sigue las mejores prácticas para el lenguaje de
    programación que hayáis consensuado, tanto en identificadores, como
    nombres de ficheros, como disposición de los diferentes ficheros en el
    repositorio? Por ejemplo, ¿todos los commits se refieren a un issue y
    aportan código para resolver el problema planteado en el mismo?
  • ¿He comprobado que el código compile, o que quien lo ha programado ha lo
    ha hecho y documentado en el repositorio?
  • ¿He añadido el PR y todos los issues que se resuelvan en él al milestone
    correspondiente, que será el primero?
  • ¿Cubre totalmente el milestone 0, o hará falta modelización adicional en
    el objetivo 4 para poder programar la lógica de negocio?
  • En resumen, ¿considero que con lo hecho puedo ponerme a trabajar
    directamente en el siguiente milestone, que incluye la lógica de negocio?

@JJ JJ mentioned this pull request Nov 21, 2023
3 tasks
@carlosservi

Copy link
Copy Markdown

Buenas, me ha tocado ser revisor. He estado mirando todo el trabajo realizado y lo veo bastante bien.

@danibarranqueroo

danibarranqueroo commented Nov 23, 2023

Copy link
Copy Markdown
Author

Buenas @JJ ya han pasado 2 días desde que lo aprobó el compañero y ningún otro revisor se ha puesto en contacto, por si puede revisarlo.

@shvtwp

shvtwp commented Nov 24, 2023

Copy link
Copy Markdown

Hola @danibarranqueroo , he estado revisando y creo que el nombre de la carpeta src debería ser lib. Además, de cara a un futuro te recomendaría echarle un vistazo a las buenas prácticas para escribir un commit. Por lo demás, no veo nada más que mejorar.

@JJ JJ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aparte de lo comentado en los issues específicos, piensa que es imposible programar la lógica de negocio si no tienes ninguna entidad que la maneje y pueda gestionar los objetos valor. Te agradecería que lo hicieras con cierta rapidez, y también contemples lo indicado por @shvtwp, porque en general es una práctica más moderna que usar src.

Comment thread iv.yaml Outdated
Es necesario una entidad que gestione Usuario y Habilidad así que creo Entrenamiento para ello
@danibarranqueroo

Copy link
Copy Markdown
Author

Gracias por la ayuda, diría que ya está listo de nuevo para revisión @JJ.

@JJ

JJ commented Nov 25, 2023

Copy link
Copy Markdown

@danibarranqueroo no puedes crear una entidad de la nada, sin un issue que la guíe, como has hecho en 5468c21 Voy a revisarlo de todas formas por la premura que corre, pero sin seguir una metodología precisa que pase de HU a través de issue al código (y documentándolo en el mensaje de commit) es prácticamente imposible hacerlo bien, y simplemente imposible evaluar si el código es correcto.

@JJ JJ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creo que hay un problema de base, que es que la HU no deja demasiado claro de qué va a ir la lógica de negocio, con lo que es muy complicado. La clave de la HU #2 es la palabra "entrenamiento" y es lo que hay que modelar, pero para una persona ajena a este mundo es muy complicado entender en qué consiste un entrenamiento.
Creo que @danibarranqueroo , a pesar de eso, ha intentado hacer un buen trabajo modelando al menos los tipos de jugadores y si hubiera seguido una metodología adecuada (con issues que describan problemas, por ejemplo, y con mensajes de commit que expliquen en qué issue se está trabajando) mejoraría bastante, pero lo esencial de este objetivo (y de todos los demás) es aplicar correctamente una metodología, así que conviene que se creen nuevos issues o se reescriban para poder seguirla correctamente.

Comment thread lib/entrenamiento.ts Outdated
Comment thread lib/habilidad.ts Outdated
@JJ

JJ commented Nov 30, 2023

Copy link
Copy Markdown

Vamos a dejar la clase entrenamiento para que la haga @raultl12 ; por favor corrige el resto. Y con un poco de prisa, por favor.

La clase entrenamiento se modelará en el objetivo 4 por lo que ahora no es necesario que la cree
Los comentarios no aportaban nada por lo que mejor se eliminan
Al eliminar el fichero entrenamiento este fichero estaba mal por lo que había que actualizarlo
Los enums que estaban en usuario y habilidad se pasan a un fichero aparte
@danibarranqueroo

Copy link
Copy Markdown
Author

Listo para revisión @JJ.

@JJ JJ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@JJ

JJ commented Dec 1, 2023

Copy link
Copy Markdown

Por favor, empieza inmediatamente con el objetivo 3 y entrégalo en los próximos días. Si puedes antes del lunes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants