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

Context providers refactor #90

Merged
merged 5 commits into from
Aug 8, 2024
Merged

Context providers refactor #90

merged 5 commits into from
Aug 8, 2024

Conversation

wjlroe
Copy link
Contributor

@wjlroe wjlroe commented Aug 6, 2024

This refactors context providers to an array of lambdas, rather than wrapping lambdas within other lambdas. This is mostly for simplicity in general - in terms of understanding, reading, debugging. It should be easier to understand the state your logger is in, because the syntax location of any context providers should point to their actual location in code, rather than the wrapping lambda that's within lib/twiglet/logger.rb. This should be backwards compatible with existing code (that's the intention). This also adds a little bit of extra test coverage (i.e. passing a context provider to the Logger initializer).

There were no tests of that code path, only using the context_provider
method.
@wjlroe wjlroe requested a review from a team as a code owner August 6, 2024 12:17
@wjlroe wjlroe requested a review from chrisholmes August 6, 2024 12:17
@wjlroe wjlroe force-pushed the context-providers-refactor branch from b55312e to 3eb4d5e Compare August 6, 2024 12:21
wjlroe added 3 commits August 6, 2024 14:24
Maintaining the existing API to the initializers and the context_provider
method. This is just simpler - without the need to wrap lambdas within
other lambdas. It should also be easier to understand and read as a
result.
@wjlroe wjlroe force-pushed the context-providers-refactor branch from 3eb4d5e to e4c3f9c Compare August 6, 2024 13:25
@wjlroe
Copy link
Contributor Author

wjlroe commented Aug 6, 2024

/dobby version minor

Copy link
Member

@addersuk addersuk left a comment

Choose a reason for hiding this comment

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

👍

@wjlroe wjlroe merged commit dde2f7c into master Aug 8, 2024
5 checks passed
@wjlroe wjlroe deleted the context-providers-refactor branch August 8, 2024 10:57
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