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

switch to os.UserHomeDir() to detect user home directory #119

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sersh88
Copy link

@sersh88 sersh88 commented Mar 9, 2020

When install shell completion scripts it's more accurate to use os.UserHomeDir().
Program can be run with sudo and will use root's homeDir instead of original user.

…ing shell completion scripts as it's more accurate (program can be run with sudo and will use root's homedir instead of original user)
@posener
Copy link
Owner

posener commented Mar 9, 2020

Looks good.
The problem is that this function was introduced in go1.12 (see failing builds).
I assume we need to split the implementation to pre go 1.12 and go 1.12+?

@codecov
Copy link

codecov bot commented Mar 10, 2020

Codecov Report

Merging #119 into master will decrease coverage by 2.47%.
The diff coverage is 71.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
- Coverage   86.56%   84.08%   -2.48%     
==========================================
  Files           8        9       +1     
  Lines         454      509      +55     
==========================================
+ Hits          393      428      +35     
- Misses         45       62      +17     
- Partials       16       19       +3     
Impacted Files Coverage Δ
flags.go 66.66% <0.00%> (ø)
internal/arg/arg.go 100.00% <ø> (ø)
predict/predict.go 100.00% <ø> (ø)
testing.go 42.85% <ø> (ø)
compflag/compflag.go 63.15% <61.97%> (-7.68%) ⬇️
predict/options.go 80.64% <80.64%> (ø)
complete.go 96.87% <100.00%> (+0.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdd345f...7912027. Read the comment docs.

@posener
Copy link
Owner

posener commented Mar 10, 2020

🙂
This is not what I meant.
In Go you can tell the compiler which go to compile in which go version using build tags.
So you can create two files, one that will be called userdir_go1_11.go that will be compiled with go 1.11 and below, and another file userdir.go for newer versions.
This files will contain an internal function that returns the user directory, but one of them will contain the old code and the other will contain the new API. Does that make sense?

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.

2 participants