Skip to content
This repository has been archived by the owner on May 9, 2021. It is now read-only.

exclude vendor dir from matching ... #320

Closed
omarkohl opened this issue Aug 25, 2017 · 10 comments
Closed

exclude vendor dir from matching ... #320

omarkohl opened this issue Aug 25, 2017 · 10 comments

Comments

@omarkohl
Copy link

Go 1.9 changed the behavior of the Go tools such as "go test" and "go vet" to ignore the vendor directory when using triple dot (...) . See golang/go#19090

How about doing the same thing for golint? It makes sense to have different Go tools behave similarly.

@omarkohl
Copy link
Author

It looks like it is addressed in #303 . Maybe this ticket could be considered as a proposal to make it the default behavior instead of an optional flag.

@nathany
Copy link

nathany commented Aug 29, 2017

Making the default behaviour to ignore vendor would match Go 1.9, which is preferable in my mind.

Here are a few reasons for making golint ./... ignore the vendor directory by default.

  1. Consistent with Go 1.9 behaviour for go test, go list, etc.
  2. As per cmd/go: exclude vendor dir from matching ... go#19090, a solution that doesn't rely on Unix-specific tools (xargs) is beneficial for Windows users.
  3. When there are an overwhelming number of lint issues from vendor, it's more likely that all lint issues will be ignored, rather than resolving lint issues within the project itself.

For example, in one project I'm working on, it looks like this:

❯ golint ./... | wc -l
    3987

Many of these lint issues may never be fixed, such as generated code for syscall.

vendor/golang.org/x/sys/unix/zsysnum_darwin_amd64.go:364:2: don't use ALL_CAPS in Go names; use CamelCase
...

If I break out some console-fu (e.g. StackOverflow) I can limit it to just the project:

❯ go list ./... | grep -v /vendor/ | xargs -L1 golint | wc -l
     183

But that isn't very user friendly, nor does it work on Windows.

/cc @dsymonds

@dlsniper
Copy link

Now that Go 1.9+ excludes vendor/ from ./... runs, maybe this can be reconsidered? Especially since go list ./... | xargs -L1 golint | wc -l does not exit with status 1 (unless further bash magic is applied). Thank you.

nstapelbroek added a commit to nstapelbroek/gatekeeper that referenced this issue Mar 4, 2018
According to golang/lint#320 (comment) golint should ignore the vendor
directory since version 1.9
@yookoala
Copy link

Any updates on the consideration?

@bfosberry
Copy link

Bump

@jjrumi
Copy link

jjrumi commented Feb 26, 2019

Not ideal but go list ./... | grep -v /vendor/ | xargs -L1 golint -set_exit_status does exit with error status

@dlsniper
Copy link

Unfortunately, that's not something that Windows users such as myself can run :(

@marcusljx
Copy link

Is there any update on this issue? I saw #325 but it seems it's been stagnant for over a year... I do have a need to maintain /vendor despite using go modules, due to legacy compatibility issues.

@nathany
Copy link

nathany commented Oct 3, 2019

It looks like work on https://go-review.googlesource.com/c/lint/+/96085 has stalled. Perhaps someone else should pick it up?

Groxx added a commit to uber/cadence that referenced this issue Jan 30, 2021
Golint is borderline abandonware (all that energy seems to be on `go vet` instead), and doesn't even ignore vendor folders from `./...` despite [having a github issue since 2017](golang/lint#320).
Plus it's non-configurable by design, which IMO encourages some bad habits.  See the next commit for details.

After a brief skim of the current state of things, `revive` popped out as a possibility, and the results are pretty good.  Runtimes:
```sh
# time make lint (before)
make lint  8.80s user 13.14s system 154% cpu 14.206 total
# time make lint (simply swapping bins)
make lint  10.85s user 16.89s system 149% cpu 18.545 total
# time make lint (new call)
make lint  21.72s user 4.22s system 2555% cpu 1.015 total
```
So that's a **14x reduction** in runtime, and now it's at the point where it's fine to run all the time (~1 second).

This default list also finds an additional 14 items, none major, and they basically match newer `golint` versions / what IDEs will often highlight:
```
# new lint, potentially worth following.
# there are many instances of this in the canary/ folder, but this was ignored earlier, so it is ignored now as well.
service/worker/failovermanager/starter.go:91:9: [context-keys-type] should not use basic type string as key in context.WithValue
# new lint, potentially worth following
service/worker/scanner/shardscanner/scanner_workflow.go:176:2: [if-return] redundant if ...; err != nil check, just return error instead.
# 3 new comment-formatting / exported-type-without-comment additions
```
Nothing significant, but (ignoring the new comment lints) possibly worth tackling, and not excessively noisy.

The following commits add some new lints + fixes, this one just aims for parity.
Groxx added a commit to uber/cadence that referenced this issue Feb 1, 2021
Golint is borderline abandonware (all that energy seems to be on `go vet` instead), and doesn't even ignore vendor folders from `./...` despite [having a github issue since 2017](golang/lint#320).
Plus it's non-configurable by design, which IMO encourages some bad habits.  See the next commit for details.

After a brief skim of the current state of things, `revive` popped out as a possibility, and the results are pretty good.  Runtimes:
```sh
# time make lint (before)
make lint  8.80s user 13.14s system 154% cpu 14.206 total
# time make lint (simply swapping bins)
make lint  10.85s user 16.89s system 149% cpu 18.545 total
# time make lint (new call)
make lint  21.72s user 4.22s system 2555% cpu 1.015 total
```
So that's a **14x reduction** in runtime, and now it's at the point where it's fine to run all the time (~1 second).

This default list also finds an additional 14 items, none major, and they basically match newer `golint` versions / what IDEs will often highlight:
```
# new lint, potentially worth following.
# there are many instances of this in the canary/ folder, but this was ignored earlier, so it is ignored now as well.
service/worker/failovermanager/starter.go:91:9: [context-keys-type] should not use basic type string as key in context.WithValue
# new lint, potentially worth following
service/worker/scanner/shardscanner/scanner_workflow.go:176:2: [if-return] redundant if ...; err != nil check, just return error instead.
# 3 new comment-formatting / exported-type-without-comment additions
```
Nothing significant, but (ignoring the new comment lints) possibly worth tackling, and not excessively noisy.

The following commits add some new lints + fixes, this one just aims for parity.
Groxx added a commit to uber/cadence that referenced this issue Feb 1, 2021
Golint is borderline abandonware (all that energy seems to be on `go vet` instead), and doesn't even ignore vendor folders from `./...` despite [having a github issue since 2017](golang/lint#320).
Plus it's non-configurable by design, which IMO encourages some bad habits.  See the next commit for details.

After a brief skim of the current state of things, `revive` popped out as a possibility, and the results are pretty good.
Runtimes on a fairly beefy machine:
```sh
# time make lint (before)
make lint  8.80s user 13.14s system 154% cpu 14.206 total
# time make lint (simply swapping bins)
make lint  10.85s user 16.89s system 149% cpu 18.545 total
# time make lint (new call)
make lint  21.72s user 4.22s system 2555% cpu 1.015 total
```
So that's a **14x reduction** in runtime, and now it's at the point where it's fine to run all the time (~1 second).
Locally, perhaps due to having fewer cores, this goes from 14s -> 2s.  Still substantially better, and still comfortably fast.

This default list also finds an additional 14 items, none major, and they basically match newer `golint` versions / what IDEs will often highlight:
```
# new lint, potentially worth following.
# there are many instances of this in the canary/ folder, but this was ignored earlier, so it is ignored now as well.
service/worker/failovermanager/starter.go:91:9: [context-keys-type] should not use basic type string as key in context.WithValue
# new lint, potentially worth following
service/worker/scanner/shardscanner/scanner_workflow.go:176:2: [if-return] redundant if ...; err != nil check, just return error instead.
# 3 new comment-formatting / exported-type-without-comment additions
```
Nothing significant, but (ignoring the new comment lints) possibly worth tackling, and not excessively noisy.

The following commits add some new lints + fixes, this one just aims for parity.
github-actions bot pushed a commit to uber/cadence that referenced this issue Feb 1, 2021
Golint is borderline abandonware (all that energy seems to be on `go vet` instead), and doesn't even ignore vendor folders from `./...` despite [having a github issue since 2017](golang/lint#320).
Plus it's non-configurable by design, which IMO encourages some bad habits.  See the next commit for details.

After a brief skim of the current state of things, `revive` popped out as a possibility, and the results are pretty good.
Runtimes on a fairly beefy machine:
```sh
# time make lint (before)
make lint  8.80s user 13.14s system 154% cpu 14.206 total
# time make lint (simply swapping bins)
make lint  10.85s user 16.89s system 149% cpu 18.545 total
# time make lint (new call)
make lint  21.72s user 4.22s system 2555% cpu 1.015 total
```
So that's a **14x reduction** in runtime, and now it's at the point where it's fine to run all the time (~1 second).
Locally, perhaps due to having fewer cores, this goes from 14s -> 2s.  Still substantially better, and still comfortably fast.

This default list also finds an additional 14 items, none major, and they basically match newer `golint` versions / what IDEs will often highlight:
```
# new lint, potentially worth following.
# there are many instances of this in the canary/ folder, but this was ignored earlier, so it is ignored now as well.
service/worker/failovermanager/starter.go:91:9: [context-keys-type] should not use basic type string as key in context.WithValue
# new lint, potentially worth following
service/worker/scanner/shardscanner/scanner_workflow.go:176:2: [if-return] redundant if ...; err != nil check, just return error instead.
# 3 new comment-formatting / exported-type-without-comment additions
```
Nothing significant, but (ignoring the new comment lints) possibly worth tackling, and not excessively noisy.

The following commits add some new lints + fixes, this one just aims for parity.
Groxx added a commit to uber/cadence that referenced this issue Feb 2, 2021
Golint is borderline abandonware (all that energy seems to be on `go vet` instead), and doesn't even ignore vendor folders from `./...` despite [having a github issue since 2017](golang/lint#320).
Plus it's non-configurable by design, which IMO encourages some bad habits.  See the next commit for details.

After a brief skim of the current state of things, `revive` popped out as a possibility, and the results are pretty good.
Runtimes on a fairly beefy machine:
```sh
# time make lint (before)
make lint  8.80s user 13.14s system 154% cpu 14.206 total
# time make lint (simply swapping bins)
make lint  10.85s user 16.89s system 149% cpu 18.545 total
# time make lint (new call)
make lint  21.72s user 4.22s system 2555% cpu 1.015 total
```
So that's a **14x reduction** in runtime, and now it's at the point where it's fine to run all the time (~1 second).
Locally, perhaps due to having fewer cores, this goes from 14s -> 2s.  Still substantially better, and still comfortably fast.

This default list also finds an additional 14 items, none major, and they basically match newer `golint` versions / what IDEs will often highlight:
```
# new lint, potentially worth following.
# there are many instances of this in the canary/ folder, but this was ignored earlier, so it is ignored now as well.
service/worker/failovermanager/starter.go:91:9: [context-keys-type] should not use basic type string as key in context.WithValue
# new lint, potentially worth following
service/worker/scanner/shardscanner/scanner_workflow.go:176:2: [if-return] redundant if ...; err != nil check, just return error instead.
# 3 new comment-formatting / exported-type-without-comment additions
```
Nothing significant, but (ignoring the new comment lints) possibly worth tackling, and not excessively noisy.

The following commits add some new lints + fixes, this one just aims for parity.
github-actions bot pushed a commit to vytautas-karpavicius/cadence that referenced this issue Feb 4, 2021
Golint is borderline abandonware (all that energy seems to be on `go vet` instead), and doesn't even ignore vendor folders from `./...` despite [having a github issue since 2017](golang/lint#320).
Plus it's non-configurable by design, which IMO encourages some bad habits.  See the next commit for details.

After a brief skim of the current state of things, `revive` popped out as a possibility, and the results are pretty good.
Runtimes on a fairly beefy machine:
```sh
# time make lint (before)
make lint  8.80s user 13.14s system 154% cpu 14.206 total
# time make lint (simply swapping bins)
make lint  10.85s user 16.89s system 149% cpu 18.545 total
# time make lint (new call)
make lint  21.72s user 4.22s system 2555% cpu 1.015 total
```
So that's a **14x reduction** in runtime, and now it's at the point where it's fine to run all the time (~1 second).
Locally, perhaps due to having fewer cores, this goes from 14s -> 2s.  Still substantially better, and still comfortably fast.

This default list also finds an additional 14 items, none major, and they basically match newer `golint` versions / what IDEs will often highlight:
```
# new lint, potentially worth following.
# there are many instances of this in the canary/ folder, but this was ignored earlier, so it is ignored now as well.
service/worker/failovermanager/starter.go:91:9: [context-keys-type] should not use basic type string as key in context.WithValue
# new lint, potentially worth following
service/worker/scanner/shardscanner/scanner_workflow.go:176:2: [if-return] redundant if ...; err != nil check, just return error instead.
# 3 new comment-formatting / exported-type-without-comment additions
```
Nothing significant, but (ignoring the new comment lints) possibly worth tackling, and not excessively noisy.

The following commits add some new lints + fixes, this one just aims for parity.
yux0 pushed a commit to yux0/cadence that referenced this issue May 4, 2021
Golint is borderline abandonware (all that energy seems to be on `go vet` instead), and doesn't even ignore vendor folders from `./...` despite [having a github issue since 2017](golang/lint#320).
Plus it's non-configurable by design, which IMO encourages some bad habits.  See the next commit for details.

After a brief skim of the current state of things, `revive` popped out as a possibility, and the results are pretty good.
Runtimes on a fairly beefy machine:
```sh
# time make lint (before)
make lint  8.80s user 13.14s system 154% cpu 14.206 total
# time make lint (simply swapping bins)
make lint  10.85s user 16.89s system 149% cpu 18.545 total
# time make lint (new call)
make lint  21.72s user 4.22s system 2555% cpu 1.015 total
```
So that's a **14x reduction** in runtime, and now it's at the point where it's fine to run all the time (~1 second).
Locally, perhaps due to having fewer cores, this goes from 14s -> 2s.  Still substantially better, and still comfortably fast.

This default list also finds an additional 14 items, none major, and they basically match newer `golint` versions / what IDEs will often highlight:
```
# new lint, potentially worth following.
# there are many instances of this in the canary/ folder, but this was ignored earlier, so it is ignored now as well.
service/worker/failovermanager/starter.go:91:9: [context-keys-type] should not use basic type string as key in context.WithValue
# new lint, potentially worth following
service/worker/scanner/shardscanner/scanner_workflow.go:176:2: [if-return] redundant if ...; err != nil check, just return error instead.
# 3 new comment-formatting / exported-type-without-comment additions
```
Nothing significant, but (ignoring the new comment lints) possibly worth tackling, and not excessively noisy.

The following commits add some new lints + fixes, this one just aims for parity.
@mvdan
Copy link
Member

mvdan commented May 8, 2021

Thank you for submitting this issue! As per golang/go#38968, we are freezing and deprecating golint. There's no drop-in replacement to golint per se, but you should find that Staticcheck works well in encouraging good Go code, much like golint did in the past, since it also includes style checks. There's always gofmt and go vet too, of course.

If you would like to contribute further, I'd encourage you to engage Staticcheck's issue tracker or look at vet's open issues, as they are both actively maintained. If you have an idea that doesn't fit into either of those tools, you could look at other Go linters, or write your own - these days it's fairly straightforward with go/analysis.

To help avoid confusion, I'm closing all issues before we freeze the repository. If you have any feedback, you can leave a comment on the proposal thread where it was decided to deprecate golint - though note that the proposal has been accepted for nearly a year. Thanks!

@mvdan mvdan closed this as completed May 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
8 participants