gofr-dev / gofr

An opinionated GoLang framework for accelerated microservice development. Built in support for databases and observability.
https://gofr.dev
Apache License 2.0
3.57k stars 233 forks source link

Help needed to enable more golangci-linter #787

Open ccoVeille opened 4 months ago

ccoVeille commented 4 months ago

Here is the list of errors I was able to detect via golangci-lint by enabling a lot of rules

951 issues:
* contextcheck: 12
* depguard: 2
* errorlint: 1
* gci: 22
* mirror: 1
* sqlclosecheck: 12
* testifylint: 271
* thelper: 16
* usestdlibvars: 19
* revive: 595
    425 use-any
     89 unused-receiver
     31 import-alias-naming
     24 unhandled-error
      5 unchecked-type-assertion
      5 import-shadowing
      3 early-return
      3 defer
      3 comment-spacings
      2 deep-exit
      2 confusing-naming
      1 modifies-value-receiver
      1 bool-literal-in-expr
      1 bare-return

Please let me know if you want me to help you to add them, I can work on a dedicated file and a dedicated GHA, so it won't cause any problem when merged

iamgoroot commented 4 months ago

There are some really important lints that should be enabled. Especially after looking into the code... Can this be enabled for new code for now? I think current issues can be taken care of later and it seems like gofr has "just throw in support of everything you can think of" in backlog so new code should definitely be checked

ccoVeille commented 4 months ago

Yes, I can enable them for new code only, that's pretty easy I did it for clipse project

aryanmehrotra commented 4 months ago

Minor linters can be fixed in a single PR, but the larger one's needed to be added separately. We would prefer adding with the fix/

ccoVeille commented 4 months ago

788 addresses a part of the issues, once merged more changes will be needed

ccoVeille commented 4 months ago

I will create a PR about using testifylint with updated code, not whole codebase

ccoVeille commented 4 months ago

Once #829 will be merged, here will be the remaining errors

93 issues:

Here is the detail for the 69 errors reported by revive: 31 revive.import-alias-naming 24 revive.unhandled-error 5 revive.unchecked-type-assertion 5 revive.import-shadowing 2 revive.confusing-naming 1 revive.redundant-import-alias 1 revive.modifies-value-receiver

srijan-27 commented 2 months ago

@ccoVeille - should I create a PR to add the above specified 3 linters?

ccoVeille commented 2 months ago

Yes, please.

I think they should be addressed in separate PR to ease the review

Some might be obvious and can be grouped, some might require large code rewrite.

You will see by working on them.

srijan-27 commented 2 months ago

Sure, yeah I am aware of changes.. just wanted to confirm if you worked on it.