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

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

Closed
silenceper opened this issue Sep 14, 2020 · 14 comments · Fixed by #1155
Closed

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

silenceper opened this issue Sep 14, 2020 · 14 comments · Fixed by #1155
Labels
enhancement New feature or request

Comments

@silenceper
Copy link
Contributor

silenceper commented Sep 14, 2020

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.

@silenceper silenceper added the bug Something isn't working label Sep 14, 2020
@silenceper silenceper changed the title [Suggestion]For better code quality, is there any plan to introduce golangci.run as a github action. [Suggestion]is there any plan to introduce golangci.run as a github action? Sep 14, 2020
@tomkerkhove tomkerkhove added enhancement New feature or request and removed bug Something isn't working labels Sep 14, 2020
@turbaszek
Copy link
Contributor

@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
Copy link
Contributor Author

@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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

silenceper commented Sep 14, 2020

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
Copy link
Contributor

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 #1088 (comment)

@silenceper
Copy link
Contributor Author

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
Copy link
Contributor Author

linked this pr #1155

@tomkerkhove
Copy link
Member

How does this relate to our recent linting @turbaszek ?

@silenceper silenceper mentioned this issue Sep 14, 2020
4 tasks
@turbaszek
Copy link
Contributor

turbaszek commented Sep 14, 2020

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
Copy link
Contributor Author

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
Copy link
Contributor

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

@silenceper
Copy link
Contributor Author

@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)

SpiritZhou pushed a commit to SpiritZhou/keda that referenced this issue Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants