Skip to content

Context provider#109

Merged
vjik merged 11 commits intomasterfrom
context-enricher
Jun 17, 2024
Merged

Context provider#109
vjik merged 11 commits intomasterfrom
context-enricher

Conversation

@vjik
Copy link
Copy Markdown
Member

@vjik vjik commented Jun 13, 2024

Q A
Is bugfix?
New feature? ✔️
Breaks BC?
Fixed issues -

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.53%. Comparing base (ae4f50b) to head (3c6b737).

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #109      +/-   ##
============================================
+ Coverage     99.49%   99.53%   +0.03%     
- Complexity      141      152      +11     
============================================
  Files             8       10       +2     
  Lines           394      427      +33     
============================================
+ Hits            392      425      +33     
  Misses            2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vjik vjik added the status:code review The pull request needs review. label Jun 13, 2024
@vjik vjik requested a review from a team June 13, 2024 12:25
Comment thread src/ContextEnricher/ContextEnricher.php Outdated
Comment thread src/ContextEnricher/ContextEnricher.php
Comment thread CHANGELOG.md Outdated
- New #108: Support of nested values in message templates' variables, e. g. `{foo.bar}` (@vjik)
- Bug #89: Fix error on parse messages, that contains variables that cannot cast to a string (@vjik)
- New #109: Add context enricher (@vjik)
- Chg #109: Deprecate `Logger` methods `setTraceLevel()` and `setExcludedTracePaths()` in favor of context enricher
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should README be adjusted?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Currently README don't contain information about this methods.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes. Bun I think it should be in separated section "Context providers".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

Comment thread src/Logger.php Outdated
@vjik vjik changed the title Context enricher Context provider Jun 14, 2024
@vjik vjik requested review from rustamwin and samdark June 14, 2024 07:25
Comment on lines +26 to +28
foreach ($this->providers as $provider) {
$context = array_merge($context, $provider->getContext());
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

@vjik vjik requested a review from samdark June 17, 2024 07:27
@vjik vjik merged commit 77c5ad3 into master Jun 17, 2024
@vjik vjik deleted the context-enricher branch June 17, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:code review The pull request needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants