Conversation
|
D:\a\sentry-native\sentry-native\src\sentry_envelope.c(765,53): error : implicit conversion changes signedness: 'long long' to 'unsigned long long' [-Werror,-Wsign-conversion] [C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\cmake0\sentry.vcxproj] D:\a\sentry-native\sentry-native\src\sentry_envelope.c(786,62): error : implicit conversion changes signedness: 'long long' to 'unsigned long long' [-Werror,-Wsign-conversion] [C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\cmake0\sentry.vcxproj] D:\a\sentry-native\sentry-native\src\sentry_envelope.c(803,61): error : implicit conversion changes signedness: 'long long' to 'unsigned long long' [-Werror,-Wsign-conversion] [C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\cmake0\sentry.vcxproj]
a051d7d to
a9bf358
Compare
vaind
left a comment
There was a problem hiding this comment.
Looks alright to me except for a few potential issues below.
Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>
|
Thanks, good catches! 🙏 I added a bunch of invalid test cases, and also the examples listed at https://develop.sentry.dev/sdk/data-model/envelopes/#full-examples |
|
@sentry review |
might make cursor satisfied?
vaind
left a comment
There was a problem hiding this comment.
Thanks for the changes, the code is much more defensive now
7f7971b to
fb54090
Compare
JoshuaMoelans
left a comment
There was a problem hiding this comment.
LGTM! do you think it makes sense to merge this before merging in the int64 PR (which is currently tied to Logs, but could be merged separately before fully finishing the Logs PR), or should we already have int64 values in this PR?
I'd vote for merging |
|
I'll fix the TODOs when |
Expose minimal envelope API for crash reporters / feedback handlers. Extracted from #1303.
Semi-pseudo example:
Minimal but complete feedback handler used for integration tests: feedback.c