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

golint: add flag to ignore vendor dirs #303

Closed
wants to merge 1 commit into from

Conversation

dvyukov
Copy link
Member

@dvyukov dvyukov commented Jun 14, 2017

A typical usage for a project is: golint ./...
Scanning all vendored packages creates noise,
takes time and is usually not useful.

Add -vendor flag that controls inclusion of vendor
dirs during ... expansion.

Note: this changes default behavior as the default
value for the flag is false. This is debatable.
The rationale for false is:

  • go tool at tip ignores vendor during fmt and test
  • people generally don't want to change vendored
    packages, especially for style

A typical usage for a project is: golint ./...
Scanning all vendored packages creates noise,
takes time and is usually not useful.

Add -vendor flag that controls inclusion of vendor
dirs during ... expansion.

Note: this changes default behavior as the default
value for the flag is false. This is debatable.
The rationale for false is:
 - go tool at tip ignores vendor during fmt and test
 - people generally don't want to change vendored
   packages, especially for style
@mvdan
Copy link
Member

mvdan commented Jun 14, 2017

If the go tool always ignores vendor/ when walking packages, shouldn't golint do the same without a flag?

@dvyukov
Copy link
Member Author

dvyukov commented Jun 14, 2017

@mvdan On second thought, I think you are right.
go fmt/test has an escape path. It won't check vendor on go fmt ./..., but it will check vendor on go fmt ./vendor/.... I.e. it does not enter into vendor dir during walk (need to double check actual code).
This will work for golint as well. And as far as we have an escape path, not having a flag seems fine.

@mvdan
Copy link
Member

mvdan commented Jun 14, 2017

Yeah, vendor/ is only ignored when its parent is walked. This works really well without an extra flag, and it also keeps consistency between linters and the go tool itself.

@jbergstroem
Copy link

Not necessarily the way forward, but a workaround should you use glide:

$ golint $(glide novendor)

@dsnet
Copy link
Member

dsnet commented Jun 27, 2017

Change seems fine to me, but let's avoid the flag.

@mholt
Copy link

mholt commented Aug 25, 2017

Any update on this? Go 1.9 is released and golint ./... is causing all sorts of havoc in my CI logs because there's about 9000 warnings in the vendor folder... but all other go commands ignore the vendor folder.

@Horgix
Copy link

Horgix commented Aug 26, 2017

@mholt : was browsing the PRs in this repo so I'm not aware to the context of this one, but personally since golint doesn't ignore the vendor folder I'm running this:

golint `go list ./... | grep -v /vendor/`

Note the backquotes that make it run in a subshell (you could also use $(...))

@mholt
Copy link

mholt commented Aug 27, 2017

@Horgix Thanks; yeah, I have a bash script that can work around it, but it's not as pretty on Windows CI...

@xgfone
Copy link

xgfone commented Aug 28, 2017

Since golint checks the packages in vendor, the intellisense is a little slow.

I think that it should not check the packages in vendor. In general, they are not our own codes. It is similar to node_modules in NodeJS.

@nathany
Copy link

nathany commented Aug 29, 2017

In light of the changes in Go 1.9, I think the default behaviour for golint ./... should be to ignore vendor. Please see #320.

@mholt
Copy link

mholt commented Aug 29, 2017

@nathany To be clear, it looks like this PR does cause vendor to be ignored by default; the title is a bit misleading, since the flag actually "enables" vendor folder lint checking.

@dvyukov
Copy link
Member Author

dvyukov commented Aug 30, 2017

Feel free to take over.

@ifiok
Copy link

ifiok commented Apr 27, 2019

golint `go list ./... | grep -v /vendor/`

This worked for me.

@SAG786
Copy link

SAG786 commented Jan 29, 2020

golint go list ./... | grep -v /vendor/

This worked for me!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants