Set up golangci-lint linter based on gometalinter #670

ginywiny · 2021-08-07T00:14:51Z

Creating a new PR because the previous one had a weird amount of strange commit history which went over my head to squash despite the many attempts taken.

Test HIGHLY based on test-gometalinter, as it is used to lint the same files, except where golangci-lint is used instead of gometalinter.

Notes:

When golangci-lint uses the --skip-files flag, it still lints the file, but simply doesn't display the linting results.
The build fails because the revive linter has found issues with certain files. This is new as revive is a new linter being used to replace golint, as it runs more quickly. This will be fixed down the line.
Comments and TODOs from the previous gometalinter test file were kept in case new linters are being used.

What was done:

Updated make deps script to install golangci-lint
Created golangci-lint script based on test-gometalinter
Updated test script to run test-golangci-lint

purpleidea · 2021-08-09T19:38:51Z

I see:

./test.sh: line 34: ./test/test-golangci-lint.sh: Permission denied

Did you u+x the file? Where do you see the list of lint errors we expect?

ginywiny · 2021-08-09T20:00:59Z

@purpleidea Whoops, must've forgot to change that. Fixed it and squashed.
The list of errors are expected for the "revive" linter.

purpleidea · 2021-08-09T20:56:27Z

Great! Please rebase to master and force-push and let's see what happens. (I pushed some fixes.)

…etalinter

purpleidea

Patch looks good, just needs a few small changes.
Also check your $EDITOR, it's letting you save files without EOF newlines.

.golangci.yml

purpleidea · 2021-08-10T01:04:19Z

misc/make-deps.sh

@@ -157,6 +157,9 @@ go get golang.org/x/lint/golint				# for `golint`-ing
 go get golang.org/x/tools/cmd/goimports		# for fmt
 go get github.com/kevinburke/go-bindata/go-bindata	# for compiling in non golang files
 go get github.com/dvyukov/go-fuzz/go-fuzz		# for fuzzing the mcl lang bits
+# heavily recommended to use curl to install instead of go get under https://golangci-lint.run/usage/install/
+curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.41.1


why is this version hardcoded? can we use the latest version? is there a safer way to do this so we don't pipe to shell? if not, please change this so it only runs in ci.

It is recommended to use a specific version as per the installation instructions (https://golangci-lint.run/usage/install/), to avoid introducing new linter errors from new minor or major releases. However, would it still be preferred to use the latest version?

I have changed this to something else. Any thoughts of it now?

test.sh

test/test-golangci-lint.sh

purpleidea · 2021-08-10T01:06:31Z

test/test-golangci-lint.sh

+# using .golangci.yml config file settings in ROOT
+gcl='golangci-lint run'
+
+# commented out from gometalinter linter test


are these five lines of comments valid?

Added these lines in case somebody were to move over any changes from gometalinter to golangci-lint. There were some linters which were entirely commented out, whereas some were still left as TODO, since they still require some fixes. More for clarity purposes, but they can be removed.

Are they still valid?

test/test-golangci-lint.sh

frebib · 2021-09-27T21:39:33Z

.golangci.yml

+  skip-files:
+    - lang/lexer.nn.go
+    - lang/y.go
+    - bindata/bindata.go


This is/will be removed in #671 so if that's merged first, this will need to be rebased and updated

frebib · 2021-09-27T21:40:57Z

misc/make-deps.sh

@@ -157,10 +157,35 @@ go get golang.org/x/lint/golint				# for `golint`-ing
 go get golang.org/x/tools/cmd/goimports		# for fmt
 go get github.com/kevinburke/go-bindata/go-bindata	# for compiling in non golang files
 go get github.com/dvyukov/go-fuzz/go-fuzz		# for fuzzing the mcl lang bits
+
 if in_ci; then
 	go get -u gopkg.in/alecthomas/gometalinter.v1 && \
 	mv "$(dirname $(command -v gometalinter.v1))/gometalinter.v1" "$(dirname $(command -v gometalinter.v1))/gometalinter" && \


Can you remove all of the gometalinter references from here and other places in the codebase?

frebib · 2021-09-27T21:43:53Z

misc/make-deps.sh

+	# sh -s -- -b $(go env GOPATH)/bin/golangci-lint v1.42.0
+
+	# curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.42.0
+	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh > $(go env GOPATH)/bin/golangci-lint-install


I don't know what @purpleidea thinks about this, but it seems wise to run this check in Github Actions, at least when running in Github. Perhaps we can skip installation and running in GH and only do it in other CI systems? Then to provide the same functionality, drop in the relevant configuration into https://github.com/purpleidea/mgmt/blob/master/.github/workflows/test.yaml
https://golangci-lint.run/usage/install/#github-actions

To clarify (as explained on the linked page), this provides useful comments in PRs in context of where the issues are, instead of being hidden away in the actions tab

purpleidea · 2021-10-05T12:50:27Z

@ginywiny Want to rebase to newest master and fixup?

ginywiny · 2021-10-05T16:14:47Z

@purpleidea I will rebase and make a fix.

ginywiny force-pushed the feat/create_golangci_lint_test_487 branch from 42c144f to 19116af Compare August 9, 2021 20:00

test: misc: Create new linter test using golangci-lint instead of gom…

9d05a93

…etalinter

ginywiny force-pushed the feat/create_golangci_lint_test_487 branch from 19116af to 9d05a93 Compare August 10, 2021 00:17

purpleidea requested changes Aug 10, 2021

View reviewed changes

ginywiny force-pushed the feat/create_golangci_lint_test_487 branch 2 times, most recently from 0b0a333 to 62407e2 Compare August 13, 2021 02:45

ginywiny requested a review from purpleidea August 31, 2021 02:28

ginywiny force-pushed the feat/create_golangci_lint_test_487 branch 12 times, most recently from 4d5cf02 to 021b5bb Compare September 5, 2021 22:36

test: misc: Update golangci-lint test to run only in CI

5ffc591

ginywiny force-pushed the feat/create_golangci_lint_test_487 branch from 021b5bb to 5ffc591 Compare September 7, 2021 04:51

frebib reviewed Sep 27, 2021

View reviewed changes

frebib mentioned this pull request Sep 27, 2021

lib: Move to go modules with regret #671

Closed

purpleidea force-pushed the master branch 2 times, most recently from 9e8c92c to a009c52 Compare September 12, 2022 01:20

purpleidea force-pushed the master branch 2 times, most recently from 2319f4f to 04f5ba6 Compare September 12, 2022 01:44

purpleidea mentioned this pull request May 12, 2023

test: Add golangci-lint test #711

Closed

purpleidea force-pushed the master branch 4 times, most recently from fe2313c to 271a94e Compare December 4, 2023 02:35

purpleidea force-pushed the master branch from b08136d to 78e59a9 Compare January 22, 2024 20:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set up golangci-lint linter based on gometalinter #670

Set up golangci-lint linter based on gometalinter #670

ginywiny commented Aug 7, 2021

purpleidea commented Aug 9, 2021

ginywiny commented Aug 9, 2021 •

edited

purpleidea commented Aug 9, 2021

purpleidea left a comment

purpleidea Aug 10, 2021

ginywiny Aug 13, 2021 •

edited

ginywiny Aug 17, 2021

purpleidea Aug 10, 2021

ginywiny Aug 10, 2021

ginywiny Aug 17, 2021

frebib Sep 27, 2021 •

edited

frebib Sep 27, 2021

frebib Sep 27, 2021

frebib Sep 27, 2021

purpleidea commented Oct 5, 2021

ginywiny commented Oct 5, 2021

Set up golangci-lint linter based on gometalinter #670

Are you sure you want to change the base?

Set up golangci-lint linter based on gometalinter #670

Conversation

ginywiny commented Aug 7, 2021

purpleidea commented Aug 9, 2021

ginywiny commented Aug 9, 2021 • edited

purpleidea commented Aug 9, 2021

purpleidea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ginywiny Aug 13, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frebib Sep 27, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purpleidea commented Oct 5, 2021

ginywiny commented Oct 5, 2021

ginywiny commented Aug 9, 2021 •

edited

ginywiny Aug 13, 2021 •

edited

frebib Sep 27, 2021 •

edited