Keda: [Suggestion]is there any plan to introduce golangci.run as a github action?

[silenceper]

For better code quality, is there any plan to introduce golangci.run as a github action.

Use the golangci.run tool to help detect potential bugs，like #1148 .

In addition, go code can be standardized.

[turbaszek]

@silenceper would you mind sharing a link to the tool you have in mind? Currently, we use revive for static analysis of code, we tried golint but it's an abandoned project

[silenceper]

@silenceper would you mind sharing a link to the tool you have in mind? Currently, we use revive for static analysis of code, we tried golint but it's an abandoned project

I use https://github.com/golangci/golangci-lint as a code quality inspection tool (allowing custom configuration).

and there is a actions can use, https://github.com/marketplace/actions/golangci-lint

      - name: golangci-lint
        uses: golangci/golangci-lint-action@v1

[turbaszek]

It looks interesting. If I'm not mistaken it's not a linter per se but a tool to configure and run multiple independent linters?

I'm just getting familiar with golang environment and I'm super surprised by the plenty of different tools that try to address nearly the same problem 👀

[silenceper]

yes, golangci contains a series of linter:https://github.com/golangci/golangci-lint/tree/master/pkg/golinters

I am very concerned about the keda project, so I hope to participate (improve code quality, bug fixes, etc.).

[turbaszek]

I think it make sens to be able to run gofmt, govet and other linters with one command, for me this sounds like a good way to unify linting. @silenceper do you have any suggested configuration for golangci?

@tomkerkhove @zroubalik what do you think?

[silenceper]

here is a example config file:https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml

Usually I use the following configuration.
Of course, this configuration is not necessarily the most reasonable.：

# options for analysis running
run:
  # default concurrency is a available CPU number
  concurrency: 4

  # timeout for analysis, e.g. 30s, 5m, default is 1m
  timeout: 1m
linters:
  # please, do not use `enable-all`: it's deprecated and will be removed soon.
  # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
  disable-all: true
  enable:
    - bodyclose
    - deadcode
    - depguard
    - dogsled
    - dupl
    - errcheck
    - funlen
    - goconst
    - gocritic
    - gocyclo
    - gofmt
    - goimports
    - golint
    - goprintffuncname
    - gosimple
    - govet
    - ineffassign
    - interfacer
    - misspell
    - nolintlint
    - rowserrcheck
    - scopelint
    - staticcheck
    - structcheck
    - stylecheck
    - typecheck
    - unconvert
    - unparam
    - unused
    - varcheck
    - whitespace

issues:
  # Excluding configuration per-path, per-linter, per-text and per-source
  exclude-rules:
    - path: _test\.go
      linters:
        - gomnd

    # https://github.com/go-critic/go-critic/issues/926
    - linters:
        - gocritic
      text: "unnecessaryDefer:"

linters-settings:
  funlen:
    lines: 80
    statements: 40

issues:
  include:
  - EXC0002 # disable excluding of issues about comments from golint
  exclude-rules:
    - linters:
       - stylecheck
      text: "ST1000:"

[turbaszek]

issues:
  include:
  - EXC0002 # disable excluding of issues about comments from golint
  exclude-rules:
    - linters:
       - stylecheck
      text: "ST1000:"

Does this mean that you can disable some golint rules? We had a problem with this because we wanted to avoid breaking change in https://github.com/kedacore/keda/pull/1088#discussion_r484253257

[silenceper]

Yes,you can disable some rules to avoid breaking change. And I think we should try our best to write code that conforms to the go code specification.

[silenceper]

linked this pr #1155

[tomkerkhove]

How does this relate to our recent linting @turbaszek ?

[turbaszek]

How does this relate to our recent linting @turbaszek ?

As stated above, the main difference is that golangci runs different linters but is not linter on its own. I can see both pros and cons. On one hand, it seems to be more robust and configurable, on the other hand the number of checks can be problematic.

It seems that golangci has wider "adoption" (number of stars) and as @silenceper has more experience in go world than I have I would assume that this may be a better approach...

However, I see that the main linter of golagci is golint with which we had some problems (and it's abandoned project). As per discussion in #1155 golangci allows users to turn off some rules - but I think this is just on or off option, right @silenceper? Is there option to disable a rule on per line basis?

Also, we had to remember that our revive configuration is simplest possible - we can add more rules

[silenceper]

yes, Golangci supports turning on or off a single linter, and also supports disabling a rule on per line basis like.

    - linters:
       - stylecheck
      text: "ST1000:"

[turbaszek]

@silenceper can you please create issues that will address additional linters that should be introduced in the future?

[silenceper]

@turbaszek I was create issue #1158.

I think this is a good way to get more community members involved in this issue to make the keda project better(fix linter errors)