-
-
Notifications
You must be signed in to change notification settings - Fork 167
feat(types): add Response context #874
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
Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #874 +/- ##
==========================================
- Coverage 73.14% 73.13% -0.01%
==========================================
Files 64 64
Lines 7365 7366 +1
==========================================
Hits 5387 5387
- Misses 1978 1979 +1 🚀 New features to boost your workflow:
|
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.
Nice! Just a few questions:
- How is this new type supposed to be used? Does not look like we are using it in the SDK, is it something that users are just supposed to know how to set? Or, does the
into_context!
stuff take care of setting it? - Do we anticipate ever needing to add more fields to this struct?
I'll make a PR on top of this to use this in the Actix Web integration. |
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, maybe you could add an example to the changelog, though!
Adds the
Reponse
context, which can be attached to events.The implementation is adapted directly from the one in Relay, minus the
inferred_content_type
, which as documented here can technically be sent by SDKs but will always be overridden by Relay.Part of #873