Simplify log_level()
#182
Open
Simplify log_level()
#182
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
…intent with comments
|
Sorry, I just got to this PR. You are absolutely right that the return value is not documented 😞 Let me think about this one, but I'm very open to any related thoughts. |
|
IMO the additional complexity needed to support that feature is not worth it, especially due to the difficulty (as mentioned in that thread) of exactly what the object should be if there are multiple loggers on the stack. If you really want to keep it, I'm happy to update my code to do so, but my sense would be that it's not going to affect much code in practice, and the simplification will make it easier to apply performance improvements down the road. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously created a function then immediately called it. Now it just inlines the results of that function, which leads to a bunch of simplification. I think this changes suggests that we could deprecate
logger()since I doubt there's much use for it outside the package.This is technically a breaking change since it alters the return value of
log_level()and friends. But the current return value was not documented, and it seems unlikely that folks would be dependent on the specific value. Still needs a NEWS bullet though.