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

Fix all the linter errors in cmd package #2334

Merged
merged 1 commit into from
Jan 25, 2022
Merged

Fix all the linter errors in cmd package #2334

merged 1 commit into from
Jan 25, 2022

Conversation

mstoykov
Copy link
Contributor

Well most are silenced actually as a lot of them will require a lot of
code changes.

@mstoykov
Copy link
Contributor Author

mstoykov commented Jan 13, 2022

I will be posting some more fixes which will actually remove some of the problems, tomorrow.

But all of those will be in separate PR based on this one

Well most are silenced actually as a lot of them will require a lot of
code changes.
Copy link
Contributor

@codebien codebien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one tiny suggestion

@@ -55,7 +55,7 @@ var (
showCloudLogs = true
)

//nolint:funlen,gocognit,gocyclo
//nolint:funlen,gocognit,gocyclo,cyclop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//nolint:funlen,gocognit,gocyclo,cyclop
//nolint:funlen,gocognit,cyclop

gocyclo has been disabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why didn't nolintlint told me that then 🤔

Copy link
Contributor

@codebien codebien Jan 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an open question for it golangci/golangci-lint#2395

Copy link
Contributor

@yorugac yorugac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very nice! 👍
I have a somewhat related suggestion though, about version; see the comment.

cmd/version.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants