Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Scan local go mod licenses for golang packages #1645
Scan local go mod licenses for golang packages #1645
Changes from 15 commits
f399e5c
4fe1cbc
6f955d8
7b306e7
dd36f8b
53eb828
41b6724
5db49e1
cab8224
193fc15
2b13ab6
9009dba
0333860
299bd2d
d9cb99b
606bd93
5cd38ad
252b980
334e0f0
4f1da6d
911f884
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
it doesn't look like we should use the value of
homeDir
if error != nil... should we return an error instead (and remove the log statement)?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.
See above comment. I don't think we should try and recreate the
GOPATH
. Either it is set and we look there, or it is not, and we do not even try.If we get requests later that lots of people want us to determine this automatically, we always can add it later.
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.
actually, I think I like that better. If
goPath
is empty, then return don't return a resolver to search against at all.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.
According to the go docs, the default gopath is:
Why wouldn't we default to this same location if GOPATH is not explicitly set?
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.
Unfortunately, you are right. I was thinking that the logic of, "if you set this flag, we will look wherever
GOPATH
points, trusting that you know where you want us to go; if not, we will not check." But that is not aligned with the go standard, as @kzantow pointed out.That leaves us with two possibilities, based on two assumptions:
GOPATH
is not set, we do not go anywhere.GOPATH
is implicit or explicit; therefore ifGOPATH
is not set, we replicate the logicMuch as I like the first, I think the second is what people will expect. Requiring them to set the env var to enable it is sufficient for them to say, "go where GOPATH takes you, whether explicit or implicit." If you don't want us to go there, do not enable it.
I was hoping the
go
command's resolution of GOPATH was a library we could just hook into, but not such luck; it is a private func insidesrc/cmd/go/internal/cfg/
, i.e. internal.