This repository has been archived by the owner on Apr 10, 2019. It is now read-only.
Convert default linter config from separate maps to a single map of linterConfig #322
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alecthomas
reviewed
Jul 27, 2017
linters.go
Outdated
| Pattern string | ||
| InstallFrom string | ||
| PartitionStrategy partitionStrategy | ||
| IsSlow bool |
There was a problem hiding this comment.
I think this should be inverted to IsFast, as most linters (unfortunately) are slow.
There was a problem hiding this comment.
Good linters are slow, there's not a lot you can do accurately with just the AST it seems. @mvdan started some work to provide interfaces to share work here https://github.com/mvdan/lint - and dominikh's megacheck helps to reuse the go-tools collection's work.
There was a problem hiding this comment.
sounds good, will swap to IsFast
a common interface for linters sounds like an interesting idea
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
|
rebased and included the change from #325 |
dnephin
force-pushed
the
declarative-config
branch
from
6f02a0d to
6042f35
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this change it should be easier to see the properties of a specific linter.
I think this change will enable us to support a full linter definition from the config format. Instead of just a list of strings in the form of
name: spec, we could support a list of boths strings andLinterConfigobjects. That would allow a user to fully customize a linter, including setting the partition strategy.If we read the config during
--installit would also let them set anInstallFrom.