TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 105 Pull requests 11 Projects Releases 2 Activity Actions

Add logs-linter #526

Merged
fyrchik merged 3 commits from achuprov/frostfs-node:logs-linter into master 2024-09-04 19:51:01 +00:00
Conversation 10 Commits 3 Files changed 11 +14 -12
2 changed files with 14 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit ad87493c41 - Show all commits

8
.golangci.yml
View file

@ -41,10 +41,10 @@ linters-settings:
noliteral:
path: bin/external_linters.so
fyrchik commented 2023-07-26 15:04:54 +00:00
Outdated
Review
Copy link

Will my local make lint continue to work if I don't have this?
Can we restrict it to actions somehow?

Will my local `make lint` continue to work if I don't have this? Can we restrict it to actions somehow?
fyrchik commented 2023-07-28 10:42:09 +00:00
Outdated
Review
Copy link

I am not sure .bin is the right name, how about just bin (we already store protogen plugin there) or .cache (it is in the git ignore).

I am not sure `.bin` is the right name, how about just `bin` (we already store `protogen` plugin there) or `.cache` (it is in the git ignore).
👍 1
fyrchik commented 2023-08-11 09:21:43 +00:00
Outdated
Review
Copy link

Do we need to change it each time we update linter version? Can it be avoided?

Do we need to change it each time we update linter version? Can it be avoided?
original-url: git.frostfs.info/TrueCloudLab/linters.git
# settings:
# target-methods : ["reportFlushError", "reportError"]
# disable-packages: ["codes", "err", "res","exec"]
# constants-repository: "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"
settings:
dstepanov-yadro marked this conversation as resolved Outdated
dstepanov-yadro commented 2023-07-26 10:33:32 +00:00
Outdated
Review
Copy link

Invalid URL

Invalid URL
achuprov commented 2023-07-26 12:15:53 +00:00
Outdated
Review
Copy link

fixed

fixed
acid-ant commented 2023-08-08 10:23:39 +00:00
Outdated
Review
Copy link

It is better to avoid commented out code in RP, especially without comments.

It is better to avoid commented out code in RP, especially without comments.
target-methods : ["reportFlushError", "reportError"]
disable-packages: ["codes", "err", "res","exec"]
constants-package: "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"
linters:
enable:

18
Makefile
View file

@ -8,7 +8,8 @@ HUB_IMAGE ?= truecloudlab/frostfs
HUB_TAG ?= "$(shell echo ${VERSION} | sed 's/^v//')"
GO_VERSION ?= 1.21
LINT_VERSION ?= 1.53.3
LINT_VERSION ?= 1.54.0
ARCH = amd64
BIN = bin
RELEASE = release
@ -25,6 +26,7 @@ PKG_VERSION ?= $(shell echo $(VERSION) | sed "s/^v//" | \
sed "s/-/~/")-${OS_RELEASE}
OUTPUT_LINT_DIR ?= $(shell pwd)/bin
acid-ant marked this conversation as resolved Outdated
acid-ant commented 2023-07-31 12:13:04 +00:00
Outdated
Review
Copy link

Why we need this new line?

Why we need this new line?
LINT_DIR = $(OUTPUT_LINT_DIR)/golangci-lint-$(LINT_VERSION)
TMP_DIR := .cache
.PHONY: help all images dep clean fmts fmt imports test lint docker/lint
@ -138,18 +140,18 @@ lint-install:
@mkdir -p $(TMP_DIR)
dstepanov-yadro marked this conversation as resolved Outdated
dstepanov-yadro commented 2023-07-27 15:06:19 +00:00
Outdated
Review
Copy link

I think it's better to not export, just GOBIN=/path/bin go install ...

I think it's better to not export, just `GOBIN=/path/bin go install ...`
@rm -rf $(TMP_DIR)/linters
@git clone --depth 1 https://git.frostfs.info/TrueCloudLab/linters.git $(TMP_DIR)/linters
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2023-07-31 14:54:03 +00:00
Outdated
Review
Copy link

This is a common target across multiple our repos, I would leave it as is.

This is a common target across multiple our repos, I would leave it as is.
achuprov commented 2023-08-02 07:47:21 +00:00
Outdated
Review
Copy link

reverted

reverted
@make -C $(TMP_DIR)/linters lib CGO_ENABLED=1 OUT_DIR=$(OUTPUT_LINT_DIR)
@@make -C $(TMP_DIR)/linters lib CGO_ENABLED=1 OUT_DIR=$(OUTPUT_LINT_DIR)
@rm -rf $(TMP_DIR)/linters
@rmdir $(TMP_DIR) 2>/dev/null || true
@GOBIN=$(OUTPUT_LINT_DIR) go install github.com/golangci/golangci-lint/cmd/golangci-lint@v$(LINT_VERSION)
@CGO_ENABLED=1 GOBIN=$(LINT_DIR) go install github.com/golangci/golangci-lint/cmd/golangci-lint@v$(LINT_VERSION)
fyrchik commented 2023-08-11 09:23:18 +00:00
Outdated
Review
Copy link

CGO_ENABLED=1 is what we miss here.

Cloning into '.cache/linters'...
remote: Enumerating objects: 27, done.
remote: Counting objects: 100% (27/27), done.
remote: Compressing objects: 100% (23/23), done.
remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (27/27), 7.68 KiB | 7.68 MiB/s, done.
Resolving deltas: 100% (3/3), done.
make[1]: Entering directory '/repo/frostfs/node/.cache/linters'
make[1]: Leaving directory '/repo/frostfs/node/.cache/linters'
ERRO Unable to load custom analyzer noliteral:bin/lint-1.54.0/external_linters.so, plugin: not implemented
ERRO Running error: unknown linters: 'noliteral', run 'golangci-lint help linters' to see the list of supported linters
make: *** [Makefile:150: lint] Error 3
`CGO_ENABLED=1` is what we miss here. ``` Cloning into '.cache/linters'... remote: Enumerating objects: 27, done. remote: Counting objects: 100% (27/27), done. remote: Compressing objects: 100% (23/23), done. remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0 Receiving objects: 100% (27/27), 7.68 KiB | 7.68 MiB/s, done. Resolving deltas: 100% (3/3), done. make[1]: Entering directory '/repo/frostfs/node/.cache/linters' make[1]: Leaving directory '/repo/frostfs/node/.cache/linters' ERRO Unable to load custom analyzer noliteral:bin/lint-1.54.0/external_linters.so, plugin: not implemented ERRO Running error: unknown linters: 'noliteral', run 'golangci-lint help linters' to see the list of supported linters make: *** [Makefile:150: lint] Error 3 ```
# Run linters
lint:
NOLITERAL_TARGET_METHODS="reportFlushError,reportError" \
NOLITERAL_DISABLE_PACKAGES="codes,err,res,exec" \
NOLITERAL_CONSTANTS_PACKAGE="git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs" \
$(OUTPUT_LINT_DIR)/golangci-lint --timeout=10m run
@if [ ! -d "$(LINT_DIR)" ]; then \
echo "Run make lint-install"; \
exit 1; \
fi
$(LINT_DIR)/golangci-lint run
fyrchik commented 2023-08-11 09:24:30 +00:00
Outdated
Review
Copy link

Previously a timeout was different, whats the reason for change?
Anyway, we have it in the .golangci.yml, why do we need it here?

Previously a timeout was different, whats the reason for change? Anyway, we have it in the `.golangci.yml`, why do we need it here?
# Install staticcheck
staticcheck-install: