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

Reject use of export_all #290

Closed
kbaird opened this issue Oct 29, 2015 · 7 comments
Closed

Reject use of export_all #290

kbaird opened this issue Oct 29, 2015 · 7 comments

Comments

@kbaird
Copy link
Collaborator

kbaird commented Oct 29, 2015

Keeping export_all in place after initial development work is typically considered a bad practice.

A rule that complains when that is still present in a file might be useful.

@igaray
Copy link
Member

igaray commented Oct 29, 2015

👍

@elbrujohalcon
Copy link
Member

👍 and can you add a similar issue in the guidelines, please?

@kbaird
Copy link
Collaborator Author

kbaird commented Oct 29, 2015

inaka/erlang_guidelines#60, although it appears to have some errors from gadget/compiler and gadget/xref.

@bullno1
Copy link
Contributor

bullno1 commented Dec 4, 2015

I guess, using a -ifdef(TEST) to conditionally export_all should be acceptable?

@elbrujohalcon
Copy link
Member

@bullno1 you were correct somewhere else where you pointed out that the compiler itself can give you this same warning. Therefore, this rule should not be default and the implementation of this rule should be considered a very very low priority.
In any case, I wouldn't add any exception like that for ifdef, if you want to do that, just don't include this rule in your elvis.config or just ignore the module where you do that altogether.

@paulo-ferraz-oliveira
Copy link
Collaborator

I guess, using a -ifdef(TEST) to conditionally export_all should be acceptable?

I've used it and find no issue with it, unless the consumer is being smart and deploying production code with TEST defined. :D

To build on what @elbrujohalcon commented, I find this rule won't bring relevant added value. Even more so since for a few Erlang/OTP versions already, warn_export_all is on by default, so you explicitly have to turn the warning off.

@elbrujohalcon
Copy link
Member

Closing this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants