-
Notifications
You must be signed in to change notification settings - Fork 418
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
lint: fix remaining lint issues. #792
lint: fix remaining lint issues. #792
Conversation
These fix the issues identified by the k/k inspired linter configuration that we will be adding: ``` pkg/pid1/pid1.go:72:14: ST1005: error strings should not end with punctuation or newlines (stylecheck) return 0, fmt.Errorf("unhandled exit status: 0x%x\n", status) ^ pkg/pid1/pid1.go:86:21: ST1005: error strings should not end with punctuation or newlines (stylecheck) return false, 0, fmt.Errorf("wait4(): %w\n", err) ^ main.go:480:34: Error return value of `pflag.CommandLine.MarkDeprecated` is not checked (errcheck) pflag.CommandLine.MarkDeprecated("branch", "use --ref instead") ^ main.go:483:34: Error return value of `pflag.CommandLine.MarkDeprecated` is not checked (errcheck) pflag.CommandLine.MarkDeprecated("change-permissions", "use --group-write instead") ^ main.go:486:34: Error return value of `pflag.CommandLine.MarkDeprecated` is not checked (errcheck) pflag.CommandLine.MarkDeprecated("dest", "use --link instead") ^ main.go:1897:16: Error return value of `io.WriteString` is not checked (errcheck) io.WriteString(h, s) ^ main.go:555:2: ifElseChain: rewrite if-else to switch statement (gocritic) if *flDeprecatedBranch != "" && (*flDeprecatedRev == "" || *flDeprecatedRev == "HEAD") { ^ ```
@@ -552,17 +552,19 @@ func main() { | |||
handleConfigError(log, true, "ERROR: --repo must be specified") | |||
} | |||
|
|||
if *flDeprecatedBranch != "" && (*flDeprecatedRev == "" || *flDeprecatedRev == "HEAD") { | |||
switch { |
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.
I'm not sure I personally agree with this linter, but it is based on https://go.dev/doc/effective_go#switch (according to go-critic/go-critic#453)
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.
Yeah, I don't agree with this guidance as dogmatic. I prefer the else-if but it's fine. not a big deal if it shuts up the tool
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.
Thanks!
/lgtm
/approve
@@ -552,17 +552,19 @@ func main() { | |||
handleConfigError(log, true, "ERROR: --repo must be specified") | |||
} | |||
|
|||
if *flDeprecatedBranch != "" && (*flDeprecatedRev == "" || *flDeprecatedRev == "HEAD") { | |||
switch { |
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.
Yeah, I don't agree with this guidance as dogmatic. I prefer the else-if but it's fine. not a big deal if it shuts up the tool
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, thockin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
These fix the issues identified by the k/k inspired linter
configuration that we will be adding: