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

lint: improve golangci-lint interoperability #2655

Closed
wants to merge 0 commits into from

Conversation

pierreN
Copy link

@pierreN pierreN commented Jan 14, 2020

Add a new option go_metalinter_use_local_config which disables linters+options for golangci-lint so users can set them on a per project basis via a config file.

golanci-lint errorformat : handle cases when no column number, since when adding some new available linters don't provide a column number

The idea of the PR is to provide the less possible amount of options to golanci-lint so vim-go users can do whatever they want in their yaml config file.

Copy link
Collaborator

@bhcleek bhcleek left a comment

Choose a reason for hiding this comment

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

Thank you for contributing. Overall, I think this is a nice improvement. I think we can simplify it, though.

Instead of using an option to specify to use the config file, how about not setting --disable-all when the enabled linters is empty? That would simplify this code and give users this same behavior without introducing a new option to manage. 🤔

let l:default_enabled = ["govet", "golint"]
else
let l:default_enabled = []
endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Modify this and other similar changes to be more in line with usual go standards that you may already be familiar with:

let l:default_enabled = ["govet", "golint"]
if go#config#MetalinterUserLocalConfig()
  let l:default_enabled = []
endif

@pierreN
Copy link
Author

pierreN commented Jan 18, 2020

Instead of using an option to specify to use the config file, how about not setting --disable-all when the enabled linters is empty?

Yes this is indeed better, I was just not sure it was OK to change this behavior. Please have a look at the new diff. Thanks!

@bhcleek
Copy link
Collaborator

bhcleek commented Jan 20, 2020

This looks great. Thank you for simplifying!

I'll merge this after the v1.22 release.

@bhcleek
Copy link
Collaborator

bhcleek commented Feb 11, 2020

@pierreN when trying to resolve some conflicts with master, I inadvertently pushed the wrong branch to your repo. I've created #2715, which is all of your changes rebased onto master. Sorry for the trouble!

bhcleek added a commit that referenced this pull request Feb 11, 2020
@pierreN
Copy link
Author

pierreN commented Feb 11, 2020

My bad, I should have checked/referenced #2707 in this PR.

Thanks for the merge and for maintaining the awesome vim-go !

@bhcleek
Copy link
Collaborator

bhcleek commented Feb 11, 2020

That was a new PR created since this one. The conflicts were introduced when it was merged; you did everything right.

Thanks again for contributing.

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.

2 participants