-
Notifications
You must be signed in to change notification settings - Fork 86
golang lint ci v2 oadp 1.5 #2070
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
base: oadp-1.5
Are you sure you want to change the base?
Conversation
Fixes openshift#1719 upgrade to latest v1 linter which support go1.23 Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
run bin/golang-ci-lint migrate v2 linter configuration Fixes openshift#1713 Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request attempts to upgrade golangci-lint from v1.55.2 to v2.1.2 and migrate the configuration to a supposed v2 format. However, there are critical issues with this upgrade as golangci-lint v2.x does not exist.
Changes:
- Updated GOLANGCI_LINT_VERSION from v1.55.2 to v2.1.2 in Makefile
- Modified golangci-lint installation logic with version checking
- Restructured .golangci.yaml configuration file with new format and removed many essential linters
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| Makefile | Updated golangci-lint version to v2.1.2 and changed module path to include /v2, added version check logic before installation |
| .golangci.yaml | Restructured configuration with version field, moved settings under nested structure, removed critical linters, added formatters section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| linters: | ||
| disable-all: true | ||
| # TODO enable commented ones | ||
| default: none |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'linters.default' field is set to 'none', which is not a standard configuration option in golangci-lint v1.x. To disable all linters by default and only enable specific ones, use 'linters.disable-all: true' instead. The current configuration may not work as expected.
| default: none | |
| disable-all: true |
| settings: | ||
| dogsled: | ||
| max-blank-identifiers: 2 | ||
| errcheck: | ||
| check-type-assertions: true | ||
| check-blank: true | ||
| goconst: | ||
| min-len: 3 | ||
| min-occurrences: 5 | ||
| goheader: | ||
| template: |- | ||
| Copyright 2021. | ||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| govet: | ||
| enable-all: true | ||
| misspell: | ||
| locale: US | ||
| nakedret: | ||
| max-func-lines: 30 | ||
| nolintlint: | ||
| require-explanation: true | ||
| require-specific: true | ||
| allow-unused: false | ||
| revive: | ||
| rules: | ||
| - name: line-length-limit | ||
| disabled: true | ||
| - name: blank-imports | ||
| - name: dot-imports | ||
| - name: duplicated-imports | ||
| - name: import-alias-naming | ||
| - name: import-shadowing | ||
| - name: redundant-import-alias | ||
| unparam: | ||
| check-exported: true |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration structure has been changed incorrectly. In golangci-lint v1.x, linter settings should be under a top-level 'linters-settings' section, not nested under 'linters.settings'. The current structure 'linters.settings' is not valid for golangci-lint v1.x and will likely cause configuration parsing errors.
| severity: | ||
| default-severity: error | ||
| case-sensitive: false | ||
| default: error |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'severity.default' field uses 'error' instead of 'default-severity: error'. In golangci-lint v1.x configuration, the correct field name is 'default-severity', not 'default'. This should be corrected to match the v1.x configuration format.
| default: error | |
| default-severity: error |
| echo "golangci-lint $(GOLANGCI_LINT_VERSION) is already installed"; \ | ||
| else \ | ||
| echo "Installing golangci-lint $(GOLANGCI_LINT_VERSION)"; \ | ||
| $(call go-install-tool-branch,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)); \ |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The module path 'github.com/golangci/golangci-lint/v2/cmd/golangci-lint' is incorrect as golangci-lint does not have a v2 major version. The correct module path for golangci-lint v1.x is 'github.com/golangci/golangci-lint/cmd/golangci-lint'. Please update this to use the correct module path.
| $(call go-install-tool-branch,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)); \ | |
| $(call go-install-tool-branch,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)); \ |
| @@ -1,127 +1,102 @@ | |||
| # Documentation reference https://github.com/golangci/golangci-lint/blob/v1.55.2/.golangci.reference.yml | |||
| version: "2" | |||
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'version: "2"' configuration format does not exist for golangci-lint. The golangci-lint configuration file does not use a version field in this manner. This line should be removed as it's not a valid configuration option for golangci-lint v1.x.
| version: "2" |
| enable: | ||
| - asasalint | ||
| - asciicheck | ||
| - bidichk | ||
| # - bodyclose | ||
| - dogsled | ||
| # - dupword | ||
| # - durationcheck | ||
| # - errcheck | ||
| - errchkjson | ||
| - exportloopref | ||
| - gci | ||
| # - ginkgolinter | ||
| # - goconst | ||
| - gofmt | ||
| # - goheader | ||
| - goprintffuncname | ||
| # - gosec | ||
| # - gosimple | ||
| # - govet | ||
| # - ineffassign | ||
| - loggercheck | ||
| # - misspell | ||
| - nakedret | ||
| # - nilerr | ||
| # - noctx | ||
| - nolintlint | ||
| - nosprintfhostport | ||
| - revive | ||
| # - staticcheck | ||
| # - stylecheck | ||
| # - unconvert | ||
| - unparam | ||
| # - unused | ||
| - usestdlibvars |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical linters have been removed from the enabled list that were previously active. Important linters like 'errcheck' (checks for unchecked errors), 'gosimple' (simplification suggestions), 'govet' (Go's built-in vet tool), 'ineffassign' (detects ineffectual assignments), 'staticcheck' (comprehensive static analysis), 'unused' (finds unused code), 'gosec' (security checks), 'misspell' (spelling), 'gofmt' (formatting), and 'gci' (import ordering) are no longer enabled. These linters provide essential code quality and security checks. Consider re-enabling these linters unless there is a specific reason to disable them.
| exclusions: | ||
| generated: lax | ||
| rules: | ||
| - linters: | ||
| - revive | ||
| text: '^struct-tag: unknown option ''inline'' in JSON tag$' | ||
| - linters: | ||
| - staticcheck | ||
| text: 'ST1000:|ST1020:|ST1021:|ST1022:' | ||
| paths: | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'linters.exclusions' section is not a valid configuration structure in golangci-lint v1.x. Exclusion rules should be under 'issues.exclude-rules' or 'issues.exclude-dirs' at the top level, not nested under 'linters.exclusions'. This configuration will not work as intended.
| formatters: | ||
| enable: | ||
| - gci | ||
| - gofmt | ||
| settings: | ||
| gci: | ||
| sections: | ||
| - standard | ||
| - default | ||
| - prefix(github.com/openshift/oadp-operator) | ||
| gofmt: | ||
| simplify: true | ||
| exclusions: | ||
| generated: lax | ||
| paths: | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'formatters' section is not a standard golangci-lint configuration section in v1.x. Formatters like 'gci' and 'gofmt' should be enabled as linters in the 'linters.enable' list, not in a separate 'formatters' section. This configuration structure will not be recognized by golangci-lint v1.x.
| errcheck: | ||
| check-type-assertions: true | ||
| check-blank: true | ||
| goconst: | ||
| min-len: 3 | ||
| min-occurrences: 5 | ||
| goheader: | ||
| template: |- | ||
| Copyright 2021. | ||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| govet: | ||
| enable-all: true | ||
| misspell: | ||
| locale: US |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Settings for 'errcheck', 'goconst', 'goheader', 'govet', and 'misspell' linters are configured, but these linters are not enabled in the 'linters.enable' list. These settings will have no effect unless the corresponding linters are also enabled. Either remove these unused settings or add the linters to the enabled list.
|
/retest |
|
@kaovilai: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
base branch test fix #2071 (review) |
Why the changes were made
Fixes #1970 (comment)
CP #1721
How to test the changes made