-
Notifications
You must be signed in to change notification settings - Fork 17
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
F/ci undockerize #366
F/ci undockerize #366
Conversation
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.
LGTM!
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.
Bueeen PR! Se ve bien, dejo aprobado con un comentario chico
copy_file '../assets/Dockerfile.ci', 'Dockerfile.ci' | ||
template '../assets/.circleci/config.yml.erb', '.circleci/config.yml' | ||
|
||
template '../assets/bin/cibuild.erb', 'bin/cibuild' | ||
run "chmod a+x bin/cibuild" | ||
|
||
copy_file '../assets/docker-compose.ci.yml', 'docker-compose.ci.yml' |
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.
Se podrían borrar el Dockerfile.ci
, bin/cibuild
y docker-compose.ci.yml
de los assets
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.
toda la reason
This PR closes #364 by updating the circle ci config to stop using cibuild/compose.ci/dockerfile.ci in favour of a self-contained plain config using the docker environment provided by circle ci. It uses caché for the testing job and reusable setup for both linting and testing jobs.
I also added the install method to the recipe in case its needed for older projects, so this closes #182, and updated the readme to suit the new ci flow.