-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
feat: make sdk.Context implement context.Context #10941
Conversation
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 seems good to merge.
Should there be comments on the Deadline, Done, Err
functions explaining that they are there for interface matching, and just pass through to the underlying ctx key value store?
Also as an aside can c.ctx be renamed to c.additional_data or something? I was pretty confused by the c.ctx on first pass 😅
(Neither of those comments really matter imo, and don't block merge)
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.
utACK
|
@aaronc ping |
I renamed it to baseCtx which I think is the role it plays |
Description
This:
WrapSDKContext
was done in that it doesn't use thecontext.Context
that is already insidesdk.Context
sdk.Context
to be used as acontext.Context
directlyValue
andWithValue
from Simplify context #4706 as I actually think these are useful, and trying to put everything directly intoContext
as a bag of variables rather than attaching it using something likeWithValue
makes the SDK less extensibleAuthor Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change