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

Pass --tests flag to gometalinter so that we lint test files #1563

Merged
merged 2 commits into from
Nov 6, 2017
Merged

Pass --tests flag to gometalinter so that we lint test files #1563

merged 2 commits into from
Nov 6, 2017

Conversation

horgh
Copy link
Contributor

@horgh horgh commented Nov 5, 2017

Previously we'd lint test files. In the recent PR
alecthomas/gometalinter#377, gometalinter stopped asking errcheck to
lint tests. This change makes us again lint test files which I think is
sensible.

Previously we'd lint test files. In the recent PR
alecthomas/gometalinter#377, gometalinter stopped asking errcheck to
lint tests. This change makes us again lint test files which I think is
sensible.
@horgh
Copy link
Contributor Author

horgh commented Nov 5, 2017

I thought about adding a new option for this, but I have a hard time seeing someone wanting to turn it off. Also, this apparently changed recently in the PR I mentioned. I was used to having tests linted and I thought something was broken. But I'm certainly open to adding an option for this if you think that's better!

" it means if we don't pass --tests, the linter won't run against test
" files. One example of a linter that will not run against tests if we do
" not specify this flag is errcheck.
let cmd += ["--tests"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be better to put this in the if a:autosave || empty(g:go_metalinter_command) block above.

That way people who have g:go_metalinter_command defined – which is documented as "for users who want to have a complete control over how gometalinter should be executed" – won't have this extra flag, which also addresses your comment and gives people a way to disable it if they really want to.

This avoids adding the flag if the option to set the gometalinter
command is set.
@horgh
Copy link
Contributor Author

horgh commented Nov 5, 2017

Great point! I moved it as suggested. Thanks for looking at this.

@bhcleek
Copy link
Collaborator

bhcleek commented Nov 6, 2017

LGTM

Thank you for contributing @horgh!

@bhcleek bhcleek merged commit 90384c0 into fatih:master Nov 6, 2017
bhcleek added a commit to bhcleek/vim-go that referenced this pull request Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants