emacs-eldev / eldev

Elisp development tool
https://emacs-eldev.github.io/eldev/
GNU General Public License v3.0
229 stars 17 forks source link

Disable elisp-lint by default #22

Closed sirikid closed 4 years ago

sirikid commented 4 years ago

It almost never works good in a freshly initialized project. It has a bad configuration system and/or default config. Right now it reports 3298 warnings in eldev itself, most of which are caused by misconfiguration, and the rest are also detected by other linters. I think these reasons are enough to disable elisp-lint by default.

doublep commented 4 years ago

This would break at least Cider's build, so no, it cannot be done anymore, at least not in this way. The only acceptable way would be to change the initial value of eldev-lint-default, but even about that I'd have doubts.

For what it's worth, package-lint produces over 200 warnings for me too, so I don't see how elisp-lint is much worse. Where do you draw a line?

In general, lint without arguments is supposed to run all linters Eldev supports (and this means it can be more linters in the future), which is usually not a good idea for your project. It is recommended to instead customize related variables in project's file Eldev if you care about this command. Or maybe in file ~/.eldev/config. Or just enumerate desired linters on the command line.

sirikid commented 4 years ago

For what it's worth, package-lint produces over 200 warnings for me too, so I don't see how elisp-lint is much worse. Where do you draw a line?

package-lint warnings seem real too me, like missing docstrings or no mention for arguments in them, most of elisp-lint warnings are about wrong indentation -- spaces instead of tabs! Because indent-tabs-mode is t by default and elisp-lint ignores the user configuration.

In general, lint without arguments is supposed to run all linters Eldev supports (and this means it can be more linters in the future), which is usually not a good idea for your project. It is recommended to instead customize related variables in project's file Eldev if you care about this command. Or maybe in file ~/.eldev/config. Or just enumerate desired linters on the command line.

I would like eldev to be more popular, which is impossible without so-called 'user friendliness'. Running a linter that will produce a lot of false positives definitely is not user-friendly.

doublep commented 4 years ago

OK, I think I agree after all. Especially since elisp-lint on its own is meant as all-encompassing linter, that itself includes package-lint and checkdoc, making running all of them kinda pointless, even if elisp-lint is properly configured.

However, it has to be done in such a way to preserve compatibility, i.e. by changing only eldev-lint-default. I suggest to leave meaning of t as is now (i.e.: all known), but change the initial value to :default, special-handled by eldev-lint-default-p. I guess eldev-deflinter could grow a parameter called :default (similar to what is present on eldev-defcleaner already), which would be set on everything but eldev-linter-elisp.

Would you like to amend your PR?

sirikid commented 4 years ago

Would you like to amend your PR?

~I think I'll create another one after #24 got merged.~

EDIT: In the end I just force pushed the new branch.