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.

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

Fix up docker compatibility messages #1170

Closed
wants to merge 1 commit into from

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Jul 27, 2018

Signed-off-by: Daniel J Walsh [email protected]

@vrothberg
Copy link
Member

LGTM

I assume the libpod.conf manpage is removed intentionally? A general question: as the files are renamed, wouldn't it makes to also s/podman/docker/ in the manpages to have valid references across the manpages?

@rhatdan
Copy link
Member Author

rhatdan commented Jul 27, 2018

@vrothberg No the idea is to make sure people know they are using podman, We don't want them to think they are using docker. That would be considered dishonest. We want them to know they are using podman, but if they cut and paste a command from google, or just do it by accident, or have scripts hard coded to run the Docker CLI, we want them to work without having to sed 's/docker/podman in their scripts.

@rhatdan
Copy link
Member Author

rhatdan commented Jul 27, 2018

There is no equivalent for libpod.conf in docker.

@@ -232,9 +232,7 @@ install.cni:
install.docker: docker-docs
install ${SELINUXOPT} -D -m 755 docker $(BINDIR)/docker
install ${SELINUXOPT} -d -m 755 $(MANDIR)/man1
install ${SELINUXOPT} -d -m 755 $(MANDIR)/man5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really want to remove this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, their are no man5 man pages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @vrothberg pointed out, we're currently shipping libpod.conf.5. Do we want to stop shipping that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. That is not installed here, it is installed in the standard install. This is just about installing links for Docker alias, so people accidently typing docker commands or man docker build, will get information back.

@mheon
Copy link
Member

mheon commented Jul 27, 2018

Code LGTM

@rhatdan
Copy link
Member Author

rhatdan commented Jul 27, 2018

@rh-atomic-bot r=mheon

@rh-atomic-bot
Copy link
Collaborator

📌 Commit c9a3480 has been approved by mheon

@TomSweeneyRedHat
Copy link
Member

LGTM, thx for the links 411

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit c9a3480 with merge 50fea69...

@rh-atomic-bot
Copy link
Collaborator

☀️ Test successful - status-papr
Approved by: mheon
Pushing 50fea69 to master...

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants