-
-
Notifications
You must be signed in to change notification settings - Fork 324
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
ref(ios): Move breadcrumbs conversion to pure obj-c to ensure compatibility with the swift implementation #3854
Conversation
…bility with the swift implementation
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
6e8584e | 447.10 ms | 474.71 ms | 27.61 ms |
80b2ce3 | 385.02 ms | 387.36 ms | 2.34 ms |
728164b | 414.34 ms | 449.22 ms | 34.88 ms |
15c80ab+dirty | 336.27 ms | 350.58 ms | 14.31 ms |
457e29f | 398.10 ms | 421.39 ms | 23.29 ms |
5bb8d5f | 431.21 ms | 459.40 ms | 28.19 ms |
d197b5c+dirty | 338.94 ms | 354.87 ms | 15.93 ms |
31fcca2 | 391.22 ms | 414.78 ms | 23.56 ms |
e5c9b8b | 409.02 ms | 426.66 ms | 17.64 ms |
12427f4 | 393.69 ms | 414.84 ms | 21.14 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
6e8584e | 17.73 MiB | 19.86 MiB | 2.12 MiB |
80b2ce3 | 17.73 MiB | 19.75 MiB | 2.02 MiB |
728164b | 17.73 MiB | 19.85 MiB | 2.12 MiB |
15c80ab+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
457e29f | 17.73 MiB | 19.84 MiB | 2.10 MiB |
5bb8d5f | 17.73 MiB | 19.93 MiB | 2.20 MiB |
d197b5c+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
31fcca2 | 17.73 MiB | 19.90 MiB | 2.17 MiB |
e5c9b8b | 17.73 MiB | 19.83 MiB | 2.10 MiB |
12427f4 | 17.73 MiB | 19.85 MiB | 2.12 MiB |
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!!
NSString * levelString = dict[@"level"]; | ||
SentryLevel sentryLevel; | ||
if ([levelString isEqualToString:@"fatal"]) { | ||
sentryLevel = kSentryLevelFatal; |
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.
hello @krystofwoldrich,
congrats on the 5.23.0 release tho.
i've just installed it and i'm having some errors due to these variables not being defined.
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.
Same problem for me
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.
There's now an issue to track this issue with the undeclared identifiers: #3863
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.
Thank you all for the messages and comments, we will release this shorty.
Until then please use the previous SDK release (5.22.3).
📢 Type of change
📜 Description
SentryLevel was moved to Swift in getsentry/sentry-cocoa#3963 and we can't use Swift code in
RNSentry.mm
, so I've moved breadcrumb conversion to pure Objective-C implementation.The current implementation works until updating to a release which includes getsentry/sentry-cocoa#3963 changes.
This PR makes RN implementation ready for the future release.
💚 How did you test it?
added unit tests, sample app
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps
#skip-changelog