-
-
Notifications
You must be signed in to change notification settings - Fork 371
Add SPI #5307
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
Add SPI #5307
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5307 +/- ##
=============================================
+ Coverage 92.905% 92.909% +0.003%
=============================================
Files 688 691 +3
Lines 86392 86703 +311
Branches 31229 31218 -11
=============================================
+ Hits 80263 80555 +292
- Misses 6028 6049 +21
+ Partials 101 99 -2
... and 22 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| adcc7d8 | 1225.90 ms | 1245.08 ms | 19.18 ms |
| dacf894 | 1232.32 ms | 1236.34 ms | 4.02 ms |
| 3437454 | 1254.04 ms | 1259.50 ms | 5.46 ms |
| 12e65d0 | 1232.17 ms | 1265.39 ms | 33.22 ms |
| 9f0d9e0 | 1206.55 ms | 1219.41 ms | 12.86 ms |
| 963b49c | 1244.94 ms | 1256.55 ms | 11.61 ms |
| 2283356 | 1228.77 ms | 1248.47 ms | 19.70 ms |
| e5dcbd5 | 1223.47 ms | 1243.90 ms | 20.43 ms |
| d3abae0 | 1200.36 ms | 1224.22 ms | 23.87 ms |
| 83887af | 1196.94 ms | 1206.82 ms | 9.88 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| adcc7d8 | 20.76 KiB | 426.15 KiB | 405.39 KiB |
| dacf894 | 20.76 KiB | 426.34 KiB | 405.58 KiB |
| 3437454 | 22.85 KiB | 408.87 KiB | 386.02 KiB |
| 12e65d0 | 22.30 KiB | 756.53 KiB | 734.23 KiB |
| 9f0d9e0 | 21.58 KiB | 424.28 KiB | 402.70 KiB |
| 963b49c | 22.30 KiB | 749.83 KiB | 727.53 KiB |
| 2283356 | 23.76 KiB | 820.30 KiB | 796.54 KiB |
| e5dcbd5 | 22.85 KiB | 414.09 KiB | 391.24 KiB |
| d3abae0 | 20.76 KiB | 434.92 KiB | 414.16 KiB |
| 83887af | 21.58 KiB | 419.64 KiB | 398.06 KiB |
Previous results on branch: addSPI
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a4c1e6b | 1233.20 ms | 1261.19 ms | 27.98 ms |
| ce97d7d | 1226.71 ms | 1251.76 ms | 25.04 ms |
| 9937cfc | 1221.37 ms | 1256.64 ms | 35.27 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a4c1e6b | 23.76 KiB | 823.14 KiB | 799.38 KiB |
| ce97d7d | 23.76 KiB | 831.18 KiB | 807.42 KiB |
| 9937cfc | 23.76 KiB | 831.19 KiB | 807.43 KiB |
f075ddf to
6384e08
Compare
philipphofmann
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.
Thanks, I think a short note on @spi(Private) here https://github.com/getsentry/sentry-cocoa/tree/main/develop-docs#swift-and-objective-c-interoperability would be great to explain why we need it.
| To make internal Swift code accessible to ObjC it needs to be `public`. This allows the ObjC header to be generated by Swift Package Manager. | ||
| To discourage use outside of the SDK, add `@_spi(Private)` to these declarations. This ensures they can only be used when the import also | ||
| adds the SPI attribute. |
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 for the comment @noahsmartin
Move Swift types that need to be objc compatible to public. This is required by SPM, and without SPM they were already public to any ObjC users of the SDK, SPM just unified the objc/swift visibility by requiring it also be public to Swift. To discourage use they are also annotated with
@_spi(Private)so that they cannot be directly imported to swift code.The tests used @testable for importing these previously
internaltypes, but now that they are SPI the import needs to use@_spi(Private)to access them#skip-changelog