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

run-checks,.golangci.yml: move checks from run-checks to lints #14721

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

olivercalder
Copy link
Member

Move the checks for math/rand and io/ioutil from run-checks to lints defined for golangci-lint.

Also, enable the gofmt lint, now that we're on go version 1.13+.

Move the checks for `math/rand` and `io/ioutil` from run-checks to lints
defined for golangci-lint.

Also, enable the gofmt lint, now that we're on go version 1.13+.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
@olivercalder
Copy link
Member Author

The run-checks and golangci-lint are doing 👉 👇 👈

Copy link
Contributor

@zyga zyga left a comment

Choose a reason for hiding this comment

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

LGTM but see my comment about fmt checks.

@@ -183,8 +195,7 @@ linters:
# gosimple may suggest patterns that work only with more recent Go versions
# - gosimple
- nakedret
# formatting is disabled until we move to Go 1.13
# - gofmt
- gofmt
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this nullify the need for separate fmt checks in run-checks?

Copy link
Contributor

Choose a reason for hiding this comment

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

let's double check, there's likely more linters we have in run-checks that may not be needed anymore

Copy link
Member Author

Choose a reason for hiding this comment

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

The manual one in run-checks runs with -s -d, which prints out a nice diff, whereas the one in golangci-lint only supports -s. So the former is what we're used to and is nicer.

Regarding other lints, I did check, and some of the others in run-checks apply to all files, while the golangci-lint checks only seem to apply to .go files. So technically I think we want to preserve the superset of checks in run-checks in some cases.

I don't think we need nakedret in run-checks anymore, though.

Copy link
Contributor

@bboozzoo bboozzoo left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -183,8 +195,7 @@ linters:
# gosimple may suggest patterns that work only with more recent Go versions
# - gosimple
- nakedret
# formatting is disabled until we move to Go 1.13
# - gofmt
- gofmt
Copy link
Contributor

Choose a reason for hiding this comment

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

let's double check, there's likely more linters we have in run-checks that may not be needed anymore

Copy link
Contributor

@MiguelPires MiguelPires left a comment

Choose a reason for hiding this comment

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

The check in run-checks is failing and it's picking up .golangci.yml and run-checks files (insert spiderman pointing at spiderman meme). Maybe it's because of the removal of the lowercase exception, although we could just remove the run-checks misspell check altogether since we have the other one

.golangci.yml:155:8: "PROCES" is a misspelling of "PROCESS"
.golangci.yml:156:8: "PROCESSS" is a misspelling of "PROCESSES"
cmd/snap-seccomp/main_test.go:637:21: "PROCES" is a misspelling of "PROCESS"
cmd/snap-seccomp/main_test.go:637:81: "PROCES" is a misspelling of "PROCESS"
cmd/snap-seccomp/main_test.go:638:21: "PROCESSS" is a misspelling of "PROCESSES"
cmd/snap-seccomp/main_test.go:638:83: "PROCESSS" is a misspelling of "PROCESSES"
run-checks:272:10: "PROCES" is a misspelling of "PROCESS"
run-checks:272:52: "PROCES" is a misspelling of "PROCESS"
run-checks:275:32: "PROCES" is a misspelling of "PROCESS"
run-checks:275:39: "PROCESSS" is a misspelling of "PROCESSES"

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.

4 participants