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 case for Docker command #87

Closed
wants to merge 1 commit into from

Conversation

coliss86
Copy link

@coliss86 coliss86 commented Oct 1, 2020

No description provided.

@wiredfool wiredfool added the Spam label Oct 1, 2020
@wiredfool wiredfool closed this Oct 1, 2020
@wiredfool
Copy link
Member

Please don’t leave junk PRs for digital oceans thing.

@coliss86
Copy link
Author

coliss86 commented Oct 1, 2020

I opened this PR because there is a mistake in the Dockerfile. Docker command must be written in upper case, as in the rest of this file.

This is not spam, it is about quality

@hugovk
Copy link
Member

hugovk commented Oct 1, 2020

I think this one is okay.

The instruction is not case-sensitive. However, convention is for them to be UPPERCASE to distinguish them from arguments more easily.

https://docs.docker.com/engine/reference/builder/

Are all the rest consistent in other files?

@wiredfool
Copy link
Member

No, they’re not consistent.

It’s a one word change that partially fixes a non problem with no explanation when there’s a rash of other spam PRs out there, from someone who has sent similar PRs to four unrelated repos.

On the plus side, it’s not just adding an emoji.

@coliss86
Copy link
Author

coliss86 commented Oct 1, 2020

Good to know, never saw in lower case.

At least thoses files have also lower case commands :
https://github.com/python-pillow/docker-images/blob/master/amazon-2-amd64/Dockerfile
https://github.com/python-pillow/docker-images/blob/master/centos-6-amd64/Dockerfile

Would like a PR with all command in upper case ?

What's the point with emoji ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants