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

doc: unify GOPATH #1069

Merged
merged 1 commit into from
Sep 26, 2019
Merged

doc: unify GOPATH #1069

merged 1 commit into from
Sep 26, 2019

Conversation

daixiang0
Copy link
Contributor

@daixiang0 daixiang0 commented Sep 26, 2019

All other docs use $GOPATH rather that $(go env GOPATH), unify them.

Signed-off-by: Xiang Dai 764524258@qq.com

Signed-off-by: Xiang Dai <764524258@qq.com>
@sh0rez sh0rez changed the title doc: unit GOPATH doc: unify GOPATH Sep 26, 2019
@rfratto
Copy link
Member

rfratto commented Sep 26, 2019

I'm not sure about this. I think $(go env GOPATH) is more guaranteed to work since not everyone sets GOPATH in their environment variables. Maybe we want to change the other references to $(go env GOPATH)?

Thoughts?

@sh0rez
Copy link
Member

sh0rez commented Sep 26, 2019

@rfratto thought of the same, however the following says no:

# run without an environment (env -i)
$ env -i sh

# print env
$ set 
IFS='
'
OPTIND='1'
PATH='/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'
PPID='28725'
PS1='$ '
PS2='> '
PS4='+ '
PWD='/home/tbraack/go/src/github.com/grafana/loki'

$ go env GOPATH

$

It looks like contrary to things like GOARCH, go cannot autodetect this and is not smarter than the plain environment variable

@rfratto
Copy link
Member

rfratto commented Sep 26, 2019

@sh0rez Hmm, I'm experiencing something different:

$ env -i bash
$ go env GOPATH
build cache is required, but could not be located: GOCACHE is not defined and neither $XDG_CACHE_HOME nor $HOME are defined
$ export HOME=/home/robert
$ go env GOPATH           
/home/robert/go

@sh0rez
Copy link
Member

sh0rez commented Sep 26, 2019

Don't use bash, it sources your .bashrc and whatnot again, so your env is not actually clean

Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

LGTM, let's go with consistency with $GOPATH and the rest of the internet for now.

@rfratto rfratto merged commit d59b648 into grafana:master Sep 26, 2019
@daixiang0 daixiang0 deleted the unit branch September 27, 2019 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants