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

chore(lint): use golangci-lint to call revive and misspell checker. #18145

Merged
merged 5 commits into from
Jan 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ linters:
- gocritic
- bidichk
- ineffassign
- revive
enable-all: false
disable-all: true
fast: false
Expand All @@ -28,6 +29,34 @@ linters-settings:
disabled-checks:
- ifElseChain
- singleCaseSwitch # Every time this occurred in the code, there was no other way.
revive:
ignore-generated-header: false
severity: warning
confidence: 0.8
errorCode: 1
warningCode: 1
rules:
- name: blank-imports
- name: context-as-argument
- name: context-keys-type
- name: dot-imports
- name: error-return
- name: error-strings
- name: error-naming
- name: exported
- name: if-return
- name: increment-decrement
- name: var-naming
- name: var-declaration
- name: package-comments
- name: range
- name: receiver-naming
- name: time-naming
- name: unexported-return
- name: indent-error-flow
- name: errorf
- name: duplicated-imports
- name: modifies-value-receiver

issues:
exclude-rules:
Expand Down
27 changes: 0 additions & 27 deletions .revive.toml

This file was deleted.

29 changes: 2 additions & 27 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,6 @@ help:
@echo " - generate-swagger generate the swagger spec from code comments"
@echo " - swagger-validate check if the swagger spec is valid"
@echo " - golangci-lint run golangci-lint linter"
@echo " - revive run revive linter"
@echo " - misspell check for misspellings"
@echo " - vet examines Go source code and reports suspicious constructs"
@echo " - test[\#TestSpecificName] run unit test"
@echo " - test-sqlite[\#TestSpecificName] run integration test for sqlite"
Expand Down Expand Up @@ -280,29 +278,6 @@ errcheck:
@echo "Running errcheck..."
@errcheck $(GO_PACKAGES)

.PHONY: revive
revive:
Copy link
Contributor

Choose a reason for hiding this comment

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

I could see why we would remove revive, I'm not sure if it actually could be a drop-in as I know from golangci-lint that they don't always have 100% configuration as if you have with cmd command.

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe we should integrate revive in golangci-lint?

@hash revive > /dev/null 2>&1; if [ $$? -ne 0 ]; then \
$(GO) install github.com/mgechev/revive@v1.1.2; \
fi
@revive -config .revive.toml -exclude=./vendor/... ./...

.PHONY: misspell-check
misspell-check:
@hash misspell > /dev/null 2>&1; if [ $$? -ne 0 ]; then \
$(GO) install github.com/client9/misspell/cmd/misspell@v0.3.4; \
fi
@echo "Running misspell-check..."
@$(GO) run build/code-batch-process.go misspell -error -i unknwon '{file-list}'
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that this shouldn't be touched, as it's using build/code-batch-process.go which seems to be special build made to batch this misspell action. Not sure why, maybe one of the other maintainers can shed some light on that.

Copy link
Contributor

Choose a reason for hiding this comment

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

build/code-batch-process.go is only a wrapper for CLI.

In Windows, you can not pass more than 32K bytes via CLI arguments, so thecode-batch-process.go split the file names into batches. We can just ignore the wrapper during refacotoring.

Copy link
Contributor

Choose a reason for hiding this comment

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

ps: the build/code-batch-process.go came from #17684 to fix #14438 , so do not worry about it 😊


.PHONY: misspell
misspell:
@hash misspell > /dev/null 2>&1; if [ $$? -ne 0 ]; then \
$(GO) install github.com/client9/misspell/cmd/misspell@v0.3.4; \
fi
@echo "Running go misspell..."
@$(GO) run build/code-batch-process.go misspell -w -i unknwon '{file-list}'

.PHONY: fmt-check
fmt-check:
# get all go files and run go fmt on them
Expand All @@ -320,7 +295,7 @@ checks: checks-frontend checks-backend
checks-frontend: svg-check

.PHONY: checks-backend
checks-backend: misspell-check test-vendor swagger-check swagger-validate
checks-backend: test-vendor swagger-check swagger-validate

.PHONY: lint
lint: lint-frontend lint-backend
Expand All @@ -332,7 +307,7 @@ lint-frontend: node_modules
npx editorconfig-checker templates

.PHONY: lint-backend
lint-backend: golangci-lint revive vet
lint-backend: golangci-lint vet

.PHONY: watch
watch:
Expand Down