-
Notifications
You must be signed in to change notification settings - Fork 50
chore: move baggage methods in propagation namespace #55
Conversation
Codecov Report
@@ Coverage Diff @@
## main #55 +/- ##
==========================================
+ Coverage 94.62% 94.66% +0.04%
==========================================
Files 40 40
Lines 502 506 +4
Branches 80 80
==========================================
+ Hits 475 479 +4
Misses 27 27
Continue to review full report at Codecov.
|
obecny
left a comment
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.
Because the methods will be available under propagation namespace instead of global, i think you should add info to Upgrade Guidelines, other than that lgtm
dyladan
left a comment
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.
I agree with bart you should update the upgrade guidelines. Other than that LGTM so I will approve
0d5668d to
9268926
Compare
|
@obecny I rebased + added the upgrade guideline, PTAL |
obecny
left a comment
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.
lgtm
Since we moved the span context method into
tracenamespace, i'm also proposing to move[create/get/set]Baggageinto thepropagationnamespace.We still export
baggageEntryMetadataFromStringwhich feels kinda wrong to me since it doesn't do anything apart creating a unusued object, could we have a less strict type for the metadata so we don't need this function ?