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

Proposal: warn if function name contains uppercase letters #675

Closed
sasa1977 opened this issue Jul 11, 2019 · 3 comments
Closed

Proposal: warn if function name contains uppercase letters #675

sasa1977 opened this issue Jul 11, 2019 · 3 comments

Comments

@sasa1977
Copy link
Contributor

Recently I've encountered a situation in a code review where the author named the function to_CSV. This immediately stroke me as unusual/non-idiomatic (see e.g. Plug.HTML.html_escape as a counterexample). After thinking a bit about it, I can't really come up with an example where using uppercase name would be idiomatic, so I thought of writing a credo check. I have a basic first draft (code is available here), and I wanted to see if you'd be interested in moving this check to credo? If so, let me know and I'll prep a PR where we can discuss the details.

@NobbZ
Copy link
Contributor

NobbZ commented Jul 12, 2019

Shouldn't this be handled by https://github.com/rrrene/credo/blob/master/lib/credo/check/readability/function_names.ex?

Major part of work is done in this line of code:

https://github.com/rrrene/credo/blob/master/lib/credo/code/name.ex#L102-L104

So to_CSV should already be disapproved by this check.

@sasa1977
Copy link
Contributor Author

Oh, I totally missed that because the error is not reported (I just double checked, and it's indeed not reported). Since credo didn't complain about it, I just assumed that this is not verified. It appears there is a bug in this check, so I'll close this issue and investigate further.

Thanks!

@sasa1977
Copy link
Contributor Author

FYI, I submitted a fix (and a couple of other improvements) in #676

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

2 participants