feat: allow to specify Go version #31
Conversation
|
Hello @sivchari, I hope this message finds you well. When you have a moment, could you please review this? Your feedback would be greatly appreciated. Thank you in advance! |
|
Hello @alexandear Sorry, I've missed your commits. I'll check it this week. |
| } | ||
|
|
||
| if lower { | ||
| // Do nothing because T.Setenv added in go1.17 |
There was a problem hiding this comment.
I think it's better to print the warning about specifying lower or empty version. What do you think ?
| } | ||
|
|
||
| parts := strings.Split(version, ".") | ||
| if len(parts) != 2 { |
There was a problem hiding this comment.
If 1.17.N is passed, this linter occurs an error at this line. Is this intent ?
| if err != nil { | ||
| return false, fmt.Errorf("go version major part must be a number: %w", err) | ||
| } | ||
| if major < 1 { |
There was a problem hiding this comment.
If Go become Go2 in the future (probably not), this linter must doesn't work. So I think this doesn't need. What do you think ?
There was a problem hiding this comment.
The linter might be actual even on Go2. But I can remove this check, no problem.
There was a problem hiding this comment.
Let's remove this check, thanks.
|
Hi, @alexandear |
Loading the Go version from For example, the gosec team did this in securego/gosec#1119. However, this led to a performance problem, as seen in golangci/golangci-lint#4735. |
Co-authored-by: sivchari <shibuuuu5@gmail.com>
|
I propose not adding this flag to not overcomplicate linter's code. Thank you for reviewing this PR. |
This PR adds
-goflag to specify Go version. This is needed forgolangci-lintto skip runningtenvif the Go version is lower than1.17. BecauseT.Setenvwas added ingo1.17.