Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Only install docker-ce-cli with docker-ce #798

Closed
wants to merge 1 commit into from
Closed

Only install docker-ce-cli with docker-ce #798

wants to merge 1 commit into from

Conversation

kenyon
Copy link
Contributor

@kenyon kenyon commented Feb 13, 2022

In #740 I mistakenly added code that manages the docker-ce-cli package when managing the "docker" package. The docker-ce-cli package should only be managed when also managing the docker-ce package.

Fixes #797

@kenyon
Only install docker-ce-cli with docker-ce
6fcd934 
In puppetlabs#740 I mistakenly added code that manages the docker-ce-cli package
when managing the "docker" package. The docker-ce-cli package should
only be managed when also managing the docker-ce package.

Fixes puppetlabs#797
@kenyon kenyon requested a review from a team as a code owner February 13, 2022 19:44
@puppet-community-rangefinder
Copy link

docker::install is a class

that may have no external impact to Forge modules.

This module is declared in 6 of 578 indexed public Puppetfiles.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@kenyon
Copy link
Contributor Author

kenyon commented Feb 13, 2022 •
edited
Loading

The test failures are unrelated.

@chelnak
Copy link
Contributor

chelnak commented May 4, 2022

@kenyon Happy to get this merged. Can you rebase and resolve the conflict in install.pp please?

@vchepkov
Copy link
Contributor

vchepkov commented May 4, 2022

I can rebase if @kenyon is busy. This effects RHEL instances as well

@chelnak chelnak mentioned this pull request May 4, 2022
Closed
@chelnak
Copy link
Contributor

chelnak commented May 5, 2022

What ever works for you both 👍

@vchepkov vchepkov mentioned this pull request May 5, 2022
Merged
@vchepkov
Copy link
Contributor

vchepkov commented May 5, 2022

@chelnak , rebased here #826

@chelnak
Copy link
Contributor

chelnak commented May 5, 2022

Merged.

This PR can be closed now. Thank you for properly cherry-picking the commit from @kenyon 🙂

@vchepkov
Copy link
Contributor

vchepkov commented May 5, 2022

@chelnak , I think we both made a mistake and merged to master instead of main :(

@vchepkov
Copy link
Contributor

vchepkov commented May 5, 2022

master should probably removed. I will create another PR

@chelnak
Copy link
Contributor

chelnak commented May 5, 2022

Ha! thats what I get for merging from GitHub mobile!!

Good spot 😄

@vchepkov vchepkov mentioned this pull request May 5, 2022
Merged
@vchepkov
Copy link
Contributor

vchepkov commented May 5, 2022

The third time is the charm #827

@chelnak
Copy link
Contributor

chelnak commented May 5, 2022

On topic of master branches, i've created this issue to track the clean up.

@kenyon kenyon closed this May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

using Packages from Debian Repository fails with missing docker-ce-cli package
3 participants
@kenyon @chelnak @vchepkov