-
Notifications
You must be signed in to change notification settings - Fork 237
DOC: use -p with mkdir ~/bin since might exist already #150
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
@@ -28,7 +28,7 @@ curl -LO https://storage.googleapis.com/container-diff/latest/container-diff-lin | |||
OR, if you want to avoid using sudo: | |||
|
|||
```shell | |||
curl -LO https://storage.googleapis.com/container-diff/latest/container-diff-linux-amd64 && chmod +x container-diff-linux-amd64 && mkdir $HOME/bin && export PATH=$PATH:$HOME/bin && mv container-diff-linux-amd64 $HOME/bin/container-diff | |||
curl -LO https://storage.googleapis.com/container-diff/latest/container-diff-linux-amd64 && chmod +x container-diff-linux-amd64 && mkdir -p $HOME/bin && export PATH=$PATH:$HOME/bin && mv container-diff-linux-amd64 $HOME/bin/container-diff |
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.
This change LGTM, but shouldn't we be downloading to /usr/local/bin
or something other than $HOME/bin
, which isn't standard? We can remove the $PATH modifying too, which won't work for new shells @nkubala
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.
there is a snippet for /usr/local/bin installation already (via sudo, since user might not have permissions to mess with it as well).
I am curious -- what did you mean by $PATH modifying ... won't work for new shells
?
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.
You need to add export PATH=$PATH:$HOME/bin
to your bashrc or a similar startup script, otherwise when you open a new shell, the path variable won't include your changes.
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.
ah, I just misread your statement, and by "new shells" meant really new types of shells etc, not just new shell sessions. Thanks for the explanation.
yeah -- in my case PATH already includes ~/bin ;)
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.
Yeah this is just a second option if users don't want to use sudo to copy to /usr/local/bin
or another shared directory.
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.
Oh sorry, didn't see that!
No description provided.