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

Fix golangci-lint precommit hook #100

Merged
merged 3 commits into from
Sep 17, 2024

Conversation

rsevilla87
Copy link
Member

Type of change

  • Refactor
  • New feature
  • Bug fix
  • Optimization
  • Documentation Update

Description

The previous hook wasn't working as expected as it was relying on this configuration https://github.com/golangci/golangci-lint/blob/master/.pre-commit-hooks.yaml#L1-L8 golangci-lint run --new-from-rev HEAD --fix, the hook's command basically fixes linting errors when the current code is different from HEAD (so no action is taken when changes are committed), this is not what we actually need, we want to report and hang on linting errors caused by a PR.

Also fixing some pieces of code to meet linting.

Related Tickets & Documents

  • Related Issue #
  • Closes #

@rsevilla87 rsevilla87 added the bug Something isn't working label Aug 30, 2024
@rsevilla87 rsevilla87 requested review from a team as code owners August 30, 2024 10:33
Disabling revive due to false positives:

```
$ make lint
Executing pre-commit for all files
pre-commit run --all-files
[INFO] Initializing environment for https://github.com/golangci/golangci-lint.
[INFO] Installing environment for https://github.com/golangci/golangci-lint.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
golangci-lint............................................................Failed
- hook id: golangci-lint
- exit code: 1

WARN [config_reader] The configuration option `linters.errcheck.ignore` is deprecated, please use `linters.errcheck.exclude-functions`.
crd-scale.go:32:16: unused-parameter: parameter 'cmd' seems to be unused, consider removing or renaming it as _ (revive)
                PreRun: func(cmd *cobra.Command, args []string) {
                             ^
crd-scale.go:35:33: unused-parameter: parameter 'args' seems to be unused, consider removing or renaming it as _ (revive)
                Run: func(cmd *cobra.Command, args []string) {
                                              ^
node-density-cni.go:39:16: unused-parameter: parameter 'cmd' seems to be unused, consider removing or renaming it as _ (revive)
                PreRun: func(cmd *cobra.Command, args []string) {
                             ^
egressip.go:155:16: unused-parameter: parameter 'cmd' seems to be unused, consider removing or renaming it as _ (revive)
                PreRun: func(cmd *cobra.Command, args []string) {
                             ^
node-density-cni.go:51:33: unused-parameter: parameter 'args' seems to be unused, consider removing or renaming it as _ (revive)
                Run: func(cmd *cobra.Command, args []string) {
                                              ^
egressip.go:162:33: unused-parameter: parameter 'args' seems to be unused, consider removing or renaming it as _ (revive)
                Run: func(cmd *cobra.Command, args []string) {
                                              ^

```

Signed-off-by: Raul Sevilla <[email protected]>
Copy link
Contributor

@vishnuchalla vishnuchalla left a comment

Choose a reason for hiding this comment

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

lgtm

@vishnuchalla vishnuchalla merged commit 466287d into kube-burner:main Sep 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ok-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants