-
Notifications
You must be signed in to change notification settings - Fork 311
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
Update docker compose #902
base: main
Are you sure you want to change the base?
Conversation
docker::compose is a classBreaking changes to this file WILL impact these 3 modules (exact match):docker::params is a classthat may have no external impact to Forge modules. This module is declared in 6 of 580 indexed public
|
abfdb00
to
84aef20
Compare
Looks like this intends to fix #891. |
@davidphay I've noticed a small issue while testing this PR:
It seems like the version 1.x download URL does not work for version 2.x:
On Docker Compose version 2.x the version number must be prefixed with a
|
24b2c0f
to
804a267
Compare
ae0cf2d
to
35cf47a
Compare
Thanks, I fixed the issue ! |
Wouldn’t it be a preferred way to install docker compose v2 via “docker-compose-plugin” package as described here https://docs.docker.com/compose/install/linux/#install-using-the-repository ? |
I agree on both. |
I started working on this issue too, but I am currently busy…
|
@kenyon Hello, I updated the code, is it good for you now ? |
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.
Looks OK, we'll have to see what the tests say.
@kenyon linting issue should be fixed |
We need someone from @puppetlabs to approve the GitHub Actions jobs. |
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.
It looks like rubocop has some complaints. You can run it locally with bundle exec rubocop
and probably auto-correct some offenses with bundle exec rubocop -a
.
Also, can you please update the PR title / description (first message) to give more context about the usage of plugins: changelog entries link to the corresponding PR and since it is a breaking change, it's worth providing the most relevant info in the first message to the end user without the need to read all the implementation work in the discussion 😉
@@ -9,7 +9,7 @@ | |||
|
|||
has_command(:docker, 'docker') | |||
|
|||
has_command(:dockercompose, 'docker-compose') | |||
has_command(:dockercompose, 'docker') |
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.
Since all usage of dockercompose
is now replaced by docker
, this line can be removed.
@davidphay The reviewer added some notes, would you mind to fix these two things? Would love to finally see this PR getting merged. 😊 |
@davidphay Thank you for the work you've done on this PR, it has saved me some work. I'd love to get it merged as well and I am available to assist if you don't mind. |
its become more critical that this gets merged since docker-compose v1 has become unreliable. |
Testing this code. Appears to work ok. Does generate the following warning |
This should be closed now that #975 was merged. |
Update docker compose to the latest version (2.16.0) & install it as docker plugin on linux server since compose v1 will be deprecated on June 2023.
reference: https://docs.docker.com/compose/install/linux/