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

Check inputs for log_ setters #170

Merged
merged 6 commits into from
Aug 7, 2024
Merged

Conversation

hadley
Copy link
Collaborator

@hadley hadley commented Aug 2, 2024

My most common mistake with logger is accidentally calling log_formatter(formatter_glue()). This PR causes incorrect usage to generate a clear error. That revealed a few places internally where logger wasn't using a function, which required a minor tweak to the interface of log_formatter() and friends in order to make them easily reversible.

hadley added a commit to hadley/logger that referenced this pull request Aug 6, 2024
Previously created a function then immediately called it. Now it just inlines the results of that function, which leads to a bunch of simplification.

Waiting for daroczig#170 since otherwise `catch_base_log()` ends up setting `appender()` to something other than a function.

After this can probably deprecate `logger()` since I don't think there's much use for it outside the package anyway.
@hadley hadley mentioned this pull request Aug 6, 2024
@hadley hadley marked this pull request as ready for review August 7, 2024 13:07
@hadley hadley changed the title WIP: check inputs for log_ setters Check inputs for log_ setters Aug 7, 2024
@hadley
Copy link
Collaborator Author

hadley commented Aug 7, 2024

@daroczig this is now ready for review.

Copy link
Owner

@daroczig daroczig left a comment

Choose a reason for hiding this comment

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

thank you

@daroczig daroczig merged commit e9d37c2 into daroczig:master Aug 7, 2024
14 checks passed
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