-
-
Notifications
You must be signed in to change notification settings - Fork 167
feat(types): add setters for envelope headers #868
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 #868 +/- ##
==========================================
- Coverage 73.04% 72.67% -0.37%
==========================================
Files 64 64
Lines 7256 7341 +85
==========================================
+ Hits 5300 5335 +35
- Misses 1956 2006 +50 🚀 New features to boost your workflow:
|
8d9b972
to
2c94b08
Compare
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.
Requesting changes, I noticed the sample_rand
serialization logic could result in rounding up. Also left some lower-prio and optional comments
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, thanks for addressing comments! Left a couple more small ones to address before merging
Modifies visibility where needed, and setters for envelope headers and the DSC (trace header).
Even though these are part of the public API, they are mostly going to be used internally, so I think this level of documentation is fine, especially considering we already have good docs on the structs themselves.
Stacked on: #867
Required for: #865