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

improve readability of make_clean_names #303

Closed
wants to merge 2 commits into from

Conversation

Tazinho
Copy link
Contributor

@Tazinho Tazinho commented May 20, 2019

I'd like to make some small adjustments regarding the readability of the make_clean_names() function. I think these might make it more easy to reason about current and further upcoming issues. The adjustments include:

  • Some smaller adjustments in the beginning (explicit comments and removing of the pipe)
  • Including also unused arguments of to_any_case() explicitly in the source code. This might make it easier to reason about issues, which might get solved by providing snakecase arguments to janitor users.

I'd like to make some small adjustments to the make_clean_names() function. I think these might make it more easy to reason about current and further upcoming issues in the future. The adjustments include

* Some smaller adjustments in the beginning (explicit comments and removing of the pipe)
* Including arguments of to_any_case() explicitly in the source code. This might make it easier to reason about issues, which might get solved by providing snakecase arguments to janitor users.
Changed `to_any_case()` to `snakecase::to_any_case()`
@sfirke
Copy link
Owner

sfirke commented Mar 12, 2020

@Tazinho: the overhaul of clean_names happening in #340 makes me think we should close this pull request. I think you'll like Bill's changes, among other things all of the arguments to to_any_case are exposed to clean_names via .... And we'll add examples of using abbreviation and cases like Title Case.

Want to take a look at #340 as it stands and, if it looks like it covers what you have here, close this one?

@Tazinho Tazinho closed this Mar 12, 2020
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 this pull request may close these issues.

2 participants