-
-
Notifications
You must be signed in to change notification settings - Fork 372
ref: Create SentryOptionsInternal #5698
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
c2e7085 to
92aa402
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #5698 +/- ##
=============================================
+ Coverage 86.564% 86.690% +0.126%
=============================================
Files 421 422 +1
Lines 36054 36088 +34
Branches 15342 16974 +1632
=============================================
+ Hits 31210 31285 +75
+ Misses 4803 4760 -43
- Partials 41 43 +2
... and 23 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
8596808 to
0819ac2
Compare
itaybre
left a comment
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.
Looks good, but seems like it failed to build on Xcode 14
0819ac2 to
4de6e54
Compare
4de6e54 to
50d7bad
Compare
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2481950 | 1221.04 ms | 1248.98 ms | 27.94 ms |
| a9fac2e | 1212.45 ms | 1219.67 ms | 7.22 ms |
| 99104c9 | 1224.84 ms | 1247.08 ms | 22.24 ms |
| 7148f97 | 1235.09 ms | 1258.07 ms | 22.98 ms |
| 6279992 | 1213.60 ms | 1241.38 ms | 27.79 ms |
| aa96485 | 1215.37 ms | 1234.04 ms | 18.67 ms |
| fc0757d | 1231.83 ms | 1248.98 ms | 17.15 ms |
| 7c7ac56 | 1225.90 ms | 1250.22 ms | 24.33 ms |
| 8e3a42f | 1222.90 ms | 1245.67 ms | 22.77 ms |
| 51f74d7 | 1236.31 ms | 1247.43 ms | 11.12 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2481950 | 23.74 KiB | 872.74 KiB | 849.00 KiB |
| a9fac2e | 23.75 KiB | 879.53 KiB | 855.78 KiB |
| 99104c9 | 23.75 KiB | 894.83 KiB | 871.09 KiB |
| 7148f97 | 23.75 KiB | 854.78 KiB | 831.03 KiB |
| 6279992 | 23.75 KiB | 891.03 KiB | 867.28 KiB |
| aa96485 | 23.75 KiB | 874.46 KiB | 850.71 KiB |
| fc0757d | 23.75 KiB | 850.73 KiB | 826.98 KiB |
| 7c7ac56 | 23.75 KiB | 902.49 KiB | 878.74 KiB |
| 8e3a42f | 23.75 KiB | 880.06 KiB | 856.31 KiB |
| 51f74d7 | 23.74 KiB | 874.08 KiB | 850.34 KiB |
This reverts commit de86244.
This reverts commit de86244.
This pulls all the internal code that doesn't really need to be part of SentryOptions into its own helper class. With this change the hybrid SDK options initialization logic goes through a static method on SentryOptionsInternal, so SentryOptions can be written in Swift without affecting the API that hybrid SDKs use.
While this doesn't change the public API, it does change the hybrid SDK from using
[[SentryOptions alloc] initWithDict:dictionary didFailWithError:&error]to using[SentryOptionsInternal initWithDict:dictionary didFailWithError:&error]I will follow up with hybrid SDK PRs to make this change.#skip-changelog