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

Add option to enable all lints in the command line #104

Closed
andradei opened this issue Jan 23, 2019 · 9 comments
Closed

Add option to enable all lints in the command line #104

andradei opened this issue Jan 23, 2019 · 9 comments

Comments

@andradei
Copy link

Is your feature request related to a problem? Please describe.
Having to pass a -config config.toml without being able to simply -all to get all of the lint rules.

Describe the solution you'd like
Add a -all or something similar to the command line options so a TOML config file isn't needed.

@mgechev
Copy link
Owner

mgechev commented Jan 24, 2019

I'd prefer to not introduce new command line options and keep the interface minimalistic.

What are your main concerns with using a config.toml file?

@andradei
Copy link
Author

One extra config file to keep around as opposed to an extra command-line option. This really comes down to preference and convenience. I understand your goal, feel free to close the issue if it doesn't follow it.

@mgechev
Copy link
Owner

mgechev commented Jan 24, 2019

Let's keep it open for now and collect opinions. If more folks would want to use -all flag we can enable it.

@chavacava
Copy link
Collaborator

chavacava commented Jan 25, 2019

The configuration file does not only contain the list of rules to apply but also some knobs that control revive behavior (default severity, error code to return, ...) and the particular configurations of rules like argument-limit, cyclomatic, max-public-sctructs, line-length-limit...

Today those rules will panic if there is no configuration for them (for example try to use line-length-limit without setting its argument). We can set defaults for rules' configuration values but, of course, they will be totally arbitrary thus not necessarily adapted to everyone's use cases.

To resume, if the goal is to be able to run revive without a config file then I'm afraid is not just as simple as adding a -all flag to the command line.

@narqo
Copy link

narqo commented Jan 25, 2019

This might be outside of the scope of the issue, but it would nice if revive allowed enable/disable particular rule via command line argument.

Since there is no one default name for the config file (i.e. $PRJ_ROOT/.revive.toml), it isn't clear how to configure an editor (vim+ale / vscode-go) use the same set of rules across different projects.

As an example revive -rule.exported=off would be the only thing I wanted to configure for my editor.

@andradei
Copy link
Author

andradei commented Jan 31, 2019

I'd like to say that after looking at the rules extensively, I finally understand the design choice for -config and why there is a default.toml. It isn't simply a matter of preference and changing the code, like @chavacava pointed out.

I must say I like the approach of a different .toml files for different levels and ways of linting in the current state of the project.

I'd suggest, though, and maybe I should open another issue for this, to add a file with all of the rules listed in in this page so it is easier to copy-paste for those that want to have them all on a file for rapid testing and iteration. Right now I'm going through the rules one by one and adding them to my file and the ones I don't want I comment out.

Also, it would be nice to have a standardized default path like @narqo mentioned. Maybe this should be yet another separate issue.

@akerl
Copy link

akerl commented Feb 1, 2019

I think maybe I don't understand still, but I can't even find a way to enable all checks from the config file. Is the only current way to say "run all checks" to explicitly list all of them in the config file?

@andradei
Copy link
Author

andradei commented Feb 4, 2019

@akerl Yes. The only way to configure any of the lints is via config file.

@mgechev
Copy link
Owner

mgechev commented Aug 2, 2019

Let's keep the configurability as minimalistic as possible. Closing it for now. Feel free to comment for further discussion.

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

No branches or pull requests

5 participants