-
Notifications
You must be signed in to change notification settings - Fork 31
add ensure_ascii param #188
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
Conversation
|
💚 CLA has been signed |
🤖 GitHub commentsJust comment with:
|
|
run docs-build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, haven't looked closely at the code but this needs some tests and some docs update in docs/setup.asciidoc in order to be merged.
…Formatter` and `StructlogFormatter` to control non-ASCII character escaping. Update documentation and add tests to verify behavior for different `ensure_ascii` values.
🔍 Preview links for changed docs |
Thanks for the feedback! I've addressed both points: Tests AddedI've added comprehensive test coverage for the For
For
All tests verify that:
Documentation UpdatedI've added documentation to
Please let me know if you'd like any changes or have additional feedback! |
basepi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Thanks for adding docs and tests!
Thanks! I've also reformatted the code to ensure consistency. The changes are now ready for review. |
feat: #170