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

Warn on resolved exceptions #164

Open
onno-vos-dev opened this issue Jul 3, 2018 · 9 comments
Open

Warn on resolved exceptions #164

onno-vos-dev opened this issue Jul 3, 2018 · 9 comments

Comments

@onno-vos-dev
Copy link
Contributor

Hi there,

I have tried to search previous issues but I cannot find this mentioned, correct me if I'm wrong.

I have been playing with an idea in my head which, before starting to work on it, I would like to discuss. Currently I'm running Elvis on a couple of large code bases and there are a bunch of modules that throw so many failures that we've had to add them to the various ignore rules since the time needed to fix them would outweigh the benefits. Occasionally during refactoring or cleanups these issues get resolved but Elvis still ignores these modules and doesn't indicate that the ignore-rule is no longer valid.

I would like to implement some form of check that can indicate whether or not ignore-rules have been resolved. Essentially even ignored-modules should be checked and if no issues are detected, some form of warning can be presented in order for the ignore-rule to be removed.

What is your stance on this feature and does it sound like something you would like to see?

@elbrujohalcon
Copy link
Member

elbrujohalcon commented Jul 3, 2018

In theory you could simply have two config files (say elvis.config and noignores.elvis.config) and periodically run with the noignores version to verify if you still need to ignore your modules, right?
You can even have an inverse.elvis.config that's configured to run only on ignored files.

But, in any case, I'm not opposed to providing a command-line option to ignore the ignores so that you can run $ elvis --no-ignores and it will disregard the ignore configs.

@onno-vos-dev
Copy link
Contributor Author

That was my kind of thinking. One of the projects that I work on is merging multiple configs from different teams into one before running elvis on the diff between the current branch and master. Creating an noignores.elvis.config or inverse.elvis.config would be quite problematic.

I'll try to implement some form of --no-ignores functionality 👍

@elbrujohalcon
Copy link
Member

elbrujohalcon commented Jul 3, 2018

Keep in mind we have ignores both at group level…

[
 {
   elvis,
   [
    {config,
     [#{dirs => ["src"],
        filter => "*.erl",
        ruleset => erl_files,
        ignore => [this_module, that_module] %% <== THIS
       },
…

…and also at rule level for some rules, e.g.

{elvis_style, invalid_dynamic_call, #{ignore => [elvis]}}

I'm sorry I can't link to proper docs (I already opened inaka/elvis#489 to fix that) :(

@elbrujohalcon
Copy link
Member

When I wrote my idea above I was thinking only about the ignore at group-level, but considering our conversation in inaka/elvis#488 I guess you also want to ignore the individual per-rule ignores, right?

@elbrujohalcon elbrujohalcon transferred this issue from inaka/elvis Jan 7, 2021
@paulo-ferraz-oliveira
Copy link
Collaborator

I got confused with so many ignores. What's the main goal?

@paulo-ferraz-oliveira
Copy link
Collaborator

As per the previous question, is this something we still need to pursue, or can the issue be closed?

@elbrujohalcon
Copy link
Member

@paulo-ferraz-oliveira what @onno-vos-dev wants is a warning that's emitted when there is an ignore rule that is effectively ignoring nothing… and therefore it's an useless ignore. Like the warning that we emit when a file/dir configuration block actually affects 0 files.

@paulo-ferraz-oliveira
Copy link
Collaborator

Oh, that makes sense. I like it. We need to keep state, though, right? At the file level, for file-level config., and at the project level, for project level config. (actually, for module, module/function and module/function/arity, also).

@elbrujohalcon
Copy link
Member

Yeah, it's not trivial by any means.

@elbrujohalcon elbrujohalcon added this to the eventually 🙄 milestone May 26, 2022
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

3 participants