Skip to content
This repository has been archived by the owner on Feb 27, 2018. It is now read-only.

Add checkEnvironment call into cmdHellInit before printExport. #366

Merged

Conversation

lalyos
Copy link
Contributor

@lalyos lalyos commented May 18, 2015

The same as cmdUp does: "Your environment variables are already set correctly." written to stderr in case no actions needed.
This way env correctness can be checked in a script by:

if [[ "$(boot2docker-cli shellinit 2>/dev/null)" == "" ]]; then
  # env is ok
else
  # env isn't correct
fi

The same as cmdUp does: "Your environment variables are already set correctly." written to stderr in case no actions needed.
This way env correctness can be checked by in script by:

if [[ "$(boot2docker-cli shellinit 2>/dev/null)" == "" ]]; then
  # env is ok
else
  # env isn't correct
fi
@tianon
Copy link
Contributor

tianon commented May 18, 2015

Seems sane IMO; LGTM

cc @Moghedrin @gmlewis

@gmlewis
Copy link
Contributor

gmlewis commented May 18, 2015

So correct me if I'm wrong, but let's say someone starts up a new shell, and it is already configured properly... then the result of running $(boot2docker-cli shellinit) would be identical to running $(echo "") which appears to be benign.
If this is the case, then LGTM.

@lalyos
Copy link
Contributor Author

lalyos commented May 18, 2015

@gmlewis To be precise $(boot2docker-cli shellinit 2>/dev/null) will be equal to $(echo "")

Its still writes to stderr

$ boot2docker-cli shellinit
Writing /Users/lpapp/.boot2docker/certs/boot2docker-vm/ca.pem
Writing /Users/lpapp/.boot2docker/certs/boot2docker-vm/cert.pem
Writing /Users/lpapp/.boot2docker/certs/boot2docker-vm/key.pem
Your environment variables are already set correctly.

@gmlewis
Copy link
Contributor

gmlewis commented May 18, 2015

LGTM. Merging.

gmlewis added a commit that referenced this pull request May 18, 2015
Add checkEnvironment call into cmdHellInit before printExport.
@gmlewis gmlewis merged commit 17e0cec into boot2docker:master May 18, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants