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

x/tools/cmd/getgo: Various comments on shell assumptions #21274

RalphCorderoy opened this issue Aug 2, 2017 · 0 comments


None yet
2 participants
Copy link

commented Aug 2, 2017

From browsing the source it uses the expansion of $HOME, but doesn't escape the value.

line := fmt.Sprintf("export %s=%s", strings.ToUpper(name), value)

It appends to .bash_profile, but bash will read .profile if .bash_profile doesn't exist, and some bash users do that so they've one file read by different shells. Actually, bash uses the first of these it finds: .bash_profile, .bash_login, .profile, so it might be .bash_login that needs modifying.

zsh reads from .zprofile, but zshConfig is .zshrc; that's for interactive shells, akin to .bashrc, and the wrong place.

Speaking of different shells, `FOO=bar; export FOO' doesn't do any harm.

I don't see why that function uppercases name each time it's used?

curl -LO$(uname)/go_installer && chmod +x
go_installer && ./go_installer && rm go_installer

When presenting this, you might want to consider wrapping it to multiple physical lines so it doesn't break awkwardly, is more readable, and users paste in the break at a harmful place. No backslashes are required.

curl -LO$(uname)/go_installer &&
chmod +x go_installer &&
./go_installer &&
rm go_installer

Re-running go_installer upgrades go, so I might like to keep it around? It's going to stay anyway if it fails.

@mikioh mikioh changed the title tools/cmd/getgo: Various comments on shell assumptions x/tools/cmd/getgo: Various comments on shell assumptions Aug 3, 2017

@gopherbot gopherbot added this to the Unreleased milestone Aug 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.