-
Notifications
You must be signed in to change notification settings - Fork 800
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
CentOS/RHEL: Use dnf if available #395
Conversation
... instead of yum. The yum command is no longer available by default in recent versions. Closes docker#394 Signed-off-by: Takashi Kajinami <kajinamit@oss.nttdata.com>
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 - looks like a sane approach, thanks!
@neersighted PTAL FWIW; one difference in This script is not designed to do updates of existing installations, but we're aware of users trying to use it for that purpose. Wondering if we should account for that 🤔 |
It looks like |
To be fair; we print a warning (and add a delay) if we detect docker was already installed to warn the user "this script might not work for you", but there's a bit of a blurry line there. Would be nice if the script worked for upgrades as well (as we know people try either way), but not sure if we can deliver any promises on that (without a full evaluation of all parts of the script). |
I agree it's best effort; still, it's somewhat trivial to add the correct flag here. I'll amend this PR. |
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
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
... instead of yum. The yum command is no longer available by default in recent versions.
Closes #394