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

New Config: ignore specific modules on Credo.Check.Readability.AliasAs #81

Open
kelvinst opened this issue Aug 5, 2022 · 3 comments · May be fixed by rrrene/credo#987
Open

New Config: ignore specific modules on Credo.Check.Readability.AliasAs #81

kelvinst opened this issue Aug 5, 2022 · 3 comments · May be fixed by rrrene/credo#987

Comments

@kelvinst
Copy link

kelvinst commented Aug 5, 2022

What do you want Credo to do?

Today the Credo.Check.Readability.AliasAs errors for any use of the :as option on aliases. My problem with that is that there are some module that you don't have control over their names and they're poorly named to not use the :as option.

In my case, specifically the module MyAppWeb.Router.Helpers from Phoenix, if I don't use as: Routes option like the suggested by phoenix generator, we should be using Helpers.page_path(conn, :index), which is a lot less clear than Routes.page_path(conn, :index).

Which existing behaviour would change?

There would not be any backwards incompatible change. The only thing would be that one would be able to set something like {Credo.Check.Readability.AliasAs, ignore: [MyAppWeb.Router.Helpers]} on .credo.exs and all modules on this list would be ignored on this check.

@kelvinst
Copy link
Author

kelvinst commented Aug 5, 2022

@rrrene I'm happy to work on it if you think that's a nice addition.

@rrrene
Copy link
Owner

rrrene commented Aug 5, 2022

I would absolutely merge this! 👍

@kelvinst
Copy link
Author

kelvinst commented Aug 6, 2022

Nice, working on it then!

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 a pull request may close this issue.

2 participants