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

Some format and syntax changes. #2533

Merged
merged 1 commit into from Nov 4, 2013

Conversation

Projects
None yet
4 participants
@jamtur01
Contributor

jamtur01 commented Nov 3, 2013

  • Added sudo as per convention to docker commands
  • Break the Dockerfile block up
  • Redis is a proper noun
  • Minor whitespace fixes
Some format and syntax changes.
* Added sudo as per convention to docker commands
* Break the Dockerfile block up
* Redis is a proper noun
* Minor whitespace fixes
@dudebout

This comment has been minimized.

Show comment
Hide comment
@dudebout

dudebout Nov 3, 2013

Contributor

Sudo is not desirable. Users should add themselves to the docker group to not have to use sudo.

Contributor

dudebout commented Nov 3, 2013

Sudo is not desirable. Users should add themselves to the docker group to not have to use sudo.

@jamtur01

This comment has been minimized.

Show comment
Hide comment
@jamtur01

jamtur01 Nov 3, 2013

Contributor

I disagree :) Also the sudo syntax is used in 90% of the documentation currently. Unless every document includes a header saying "This assumes you've added yourself to the docker group" then none of the commands will work. That violates the principal of least surprise to me.

Contributor

jamtur01 commented Nov 3, 2013

I disagree :) Also the sudo syntax is used in 90% of the documentation currently. Unless every document includes a header saying "This assumes you've added yourself to the docker group" then none of the commands will work. That violates the principal of least surprise to me.

@dudebout

This comment has been minimized.

Show comment
Hide comment
@dudebout

dudebout Nov 3, 2013

Contributor

To me this should be part of the installation process as it is right now. Then sudo should be dropped from all the other documentation places. This is noise that does not add anything.

To me, a command prefixed by sudo tells me that I cannot run it on a machine on which I do not have sudo access. I can run docker on a machine on which I do not have sudo access by having an administrator add me to the docker group.

Contributor

dudebout commented Nov 3, 2013

To me this should be part of the installation process as it is right now. Then sudo should be dropped from all the other documentation places. This is noise that does not add anything.

To me, a command prefixed by sudo tells me that I cannot run it on a machine on which I do not have sudo access. I can run docker on a machine on which I do not have sudo access by having an administrator add me to the docker group.

@dudebout

This comment has been minimized.

Show comment
Hide comment
@dudebout

dudebout Nov 3, 2013

Contributor

I do agree on being consistent and I also agree on giving the documentation a little lovin as you are doing.

Contributor

dudebout commented Nov 3, 2013

I do agree on being consistent and I also agree on giving the documentation a little lovin as you are doing.

@jamtur01

This comment has been minimized.

Show comment
Hide comment
@jamtur01

jamtur01 Nov 3, 2013

Contributor

Currently its not consistent either way - my view is that we make it consistent towards sudo, update the installation documentation and then make it all consistent towards no sudo with some notes/warnings/etc scattered where needed. YMMV and thanks for the feedback! :)

Contributor

jamtur01 commented Nov 3, 2013

Currently its not consistent either way - my view is that we make it consistent towards sudo, update the installation documentation and then make it all consistent towards no sudo with some notes/warnings/etc scattered where needed. YMMV and thanks for the feedback! :)

@jpetazzo

This comment has been minimized.

Show comment
Hide comment
@jpetazzo
Contributor

jpetazzo commented Nov 3, 2013

@ghost ghost assigned metalivedev Nov 4, 2013

@metalivedev

This comment has been minimized.

Show comment
Hide comment
@metalivedev

metalivedev Nov 4, 2013

Contributor

using sudo is the current documentation standard. It is a reminder that docker does require special privileges to run.

Contributor

metalivedev commented Nov 4, 2013

using sudo is the current documentation standard. It is a reminder that docker does require special privileges to run.

metalivedev pushed a commit that referenced this pull request Nov 4, 2013

Andy Rothfusz
Merge pull request #2533 from jamtur01/add_sudo
Some format and syntax changes.

@metalivedev metalivedev merged commit dc33387 into moby:master Nov 4, 2013

@dudebout

This comment has been minimized.

Show comment
Hide comment
@dudebout

dudebout Nov 4, 2013

Contributor

@metalivedev only the daemon does. Then I do not need it I just need to be in a group which is way way weaker than having super user access.

Contributor

dudebout commented Nov 4, 2013

@metalivedev only the daemon does. Then I do not need it I just need to be in a group which is way way weaker than having super user access.

@metalivedev

This comment has been minimized.

Show comment
Hide comment
@metalivedev

metalivedev Nov 4, 2013

Contributor

#2552 might also interest you, @jamtur01 if you're looking for a next project :-)

@dudebout the "docker" group may be equivalent to having root access on the host. Please see #1655

Contributor

metalivedev commented Nov 4, 2013

#2552 might also interest you, @jamtur01 if you're looking for a next project :-)

@dudebout the "docker" group may be equivalent to having root access on the host. Please see #1655

@jamtur01 jamtur01 deleted the jamtur01:add_sudo branch Nov 4, 2013

@jamtur01

This comment has been minimized.

Show comment
Hide comment
@jamtur01

jamtur01 Nov 4, 2013

Contributor

@metalivedev I'll tackle that after #2534 gets merged - clean start on top of that.

Contributor

jamtur01 commented Nov 4, 2013

@metalivedev I'll tackle that after #2534 gets merged - clean start on top of that.

@dudebout

This comment has been minimized.

Show comment
Hide comment
@dudebout

dudebout Nov 4, 2013

Contributor

@metalivedev Thanks for the pointer to #1655. In this case it is more reasonnable to keep sudo.

Contributor

dudebout commented Nov 4, 2013

@metalivedev Thanks for the pointer to #1655. In this case it is more reasonnable to keep sudo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment