-
Notifications
You must be signed in to change notification settings - Fork 145
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
[WIP] Bridge: Logging Bridge for SwiftLog #535
Conversation
Hello Reviewers, can you please take a look at the approach? Your feedback is appreciated. Thanks @nachoBonafonte @bryce-b @vvydier |
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.
This is looking really good, and the documentation is helping a lot. I added some comments about the API and consistency. Let me know what do you think.
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.
Just missing some tests for the code and I would consider it ready
access change to accommodate tests.
Added Tests
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #535 +/- ##
==========================================
- Coverage 68.41% 68.17% -0.24%
==========================================
Files 344 346 +2
Lines 15151 15240 +89
==========================================
+ Hits 10365 10390 +25
- Misses 4786 4850 +64 ☔ View full report in Codecov by Sentry. |
Sorry, was not able to work on this again. There is just one thing - the bridge may not want to depend on the OpenTelemetrySDK |
closing PR due to contamination of upstream repo. I'll reopen with another branch. |
@khushijain21 I removed the dependency on the SDK in the new PR |
Logging Bridge for Apple's SwiftLog #517
Some Notes:
Basic Structure - LogHandler implements
TODO: Write tests for the bridge