-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[IV-23-24]Objetivo 6 #61
base: main
Are you sure you want to change the base?
Conversation
añado apartado con los archivos de integración continua
añado configuración para poder lanzar el contenedor y ejecutar test
@JJ Listo para revisión |
¿No prefieres esperar a que lo revise alguien? Por ejemplo @PabloBarTo o @puchy22 (aunque no hayan salido aleatoriamente) |
Buenas @JJ, yo era por si quizás estaban algo ocupados ahora mismo. Aunque no pasa nada, espero alguna revisión de ellos 👍 |
Hombre, la verdad es que los que estaban asignados ya han hecho muchas revisiones y están bastante liados con sus propios objetivos, por eso he mencionado a los otros dos. Si no pueden, en un par de días o el lunes a más tardar te lo reviso. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feliz año @Christianlr ☃️ , he revisado el objetivo y por lo general muy buen trabajo. He visto que has tenido en cuenta gran variedad de herramientas y has probado distintas versiones de node y distintos entornos. Revisa las anotaciones que te he dejado y a ver como avanza, suerte 🍀.
expreso con mayor claridad puntos mencionados en la elección de herramientas CI
Se quieren lanzar test, por lo que no tiene sentido tener 'build'
Buenas @puchy22 y feliz año nuevo! ☃️ Muchas gracias por la revisión tan rápida que has hecho. Estás que no paras! He arreglado todos aquellos comentarios que has hecho. Cuando puedas, revísalo a ver que te parece! 😉 💪 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Christianlr Por mí lo veo perfecto, con los cambios ha quedado mucho más completo 👍
@JJ Pues supongo que ahora si, cuando ya puedas, está listo para revisión (por cierto feliz año para ti también! 🙏 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Queda el misterio de qué pasó con cloudbees... Y la configuración de AppVeyor, pero no son obstáculos para superar el objetivo.
- docker version | ||
|
||
build_script: | ||
- docker build -t christianlr/mibarberschedule . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
¿Por qué necesitas construirlo? ¿No te lo puedes bajar directamente del repo?
image: Ubuntu | ||
|
||
install: | ||
- docker version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Esto en realidad no hace nada...
integración continua más conveniente y se han calificado los sistemas que
cumplan esos requisitos según esos criterios?
sea necesario para que aparezca correctamente en GitHub (y se pueda comprobar
desde los tests)?
dellenguaje con las que funciona correctamente nuestra aplicación?
tanto en el sistema de CI como en el contenedor Docker?
testeando y se ha comprobado que no se comprueban varias veces lo mismo (la
misma versión en CI y en el contenedor Docker en otro sistema CI?