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

to_alias() now sanitizes string before calling tolower() #427

Merged
merged 6 commits into from
Jul 21, 2023

Conversation

jdhoffa
Copy link
Member

@jdhoffa jdhoffa commented May 25, 2023

Thanks @kalashsinghal, @Tilmon and @maurolepore for flagging this! And apologies for the delay on the solution.

Closes #425

@jdhoffa jdhoffa requested a review from cjyetman May 25, 2023 14:50
@jdhoffa
Copy link
Member Author

jdhoffa commented Jul 20, 2023

@cjyetman could you have a look at this?

@jdhoffa jdhoffa requested a review from AlexAxthelm July 20, 2023 09:27
cjyetman
cjyetman previously approved these changes Jul 20, 2023
Copy link
Member

@cjyetman cjyetman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems ok to me. the title of this PR is super confusing, and maybe not aligned with what it's doing... and the NEWS item mentions to_lower which I don't even know if such a thing exists

@jdhoffa jdhoffa changed the title to_lower() now sanitizes string before calling to_lower() to_alias() now sanitizes string before calling tolower() Jul 20, 2023
@jdhoffa
Copy link
Member Author

jdhoffa commented Jul 20, 2023

seems ok to me. the title of this PR is super confusing, and maybe not aligned with what it's doing... and the NEWS item mentions to_lower which I don't even know if such a thing exists

oof sorry that's on me, got confused between to_alias and tolower.

Should be fixed now!

@jdhoffa jdhoffa requested a review from cjyetman July 20, 2023 16:43
@jdhoffa jdhoffa merged commit c629f9e into main Jul 21, 2023
18 of 22 checks passed
@jdhoffa jdhoffa deleted the 425-handle_string_encodings branch July 21, 2023 06:41
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.

to_alias() errors with a string that tolower() can't handle
2 participants