Skip to content

Experimental options in Swift #5329

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 10, 2025
Merged

Experimental options in Swift #5329

merged 3 commits into from
Jun 10, 2025

Conversation

noahsmartin
Copy link
Contributor

As described in the SPM migration doc, this moves a Swift type from the objc interface into an extension, making it visible from Swift in SPM. It unfortunately requires the force cast, but I think is very safe and the public API does not change, which is why none of the call-sites of these properties had to change.

The objc property is readonly so we don't have to worry about it being set anywhere to an incorrect object, other than in SentryOptions.m itself which only sets it one time.

I verified the objc interface does not change using the sample app, which access this property when configuring options. It's part of the iOS-ObjectiveC sample already

#skip-changelog

@noahsmartin noahsmartin mentioned this pull request May 31, 2025
Closed
@noahsmartin noahsmartin force-pushed the experimentalOptionsOnlySwift branch from ccf3791 to 365d42e Compare May 31, 2025 01:36
@codecov Codecov
Copy link

codecov bot commented May 31, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.027%. Comparing base (9aa0b44) to head (ad32dbb).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5329       +/-   ##
=============================================
+ Coverage   86.025%   86.027%   +0.002%     
=============================================
  Files          397       397               
  Lines        34584     34589        +5     
  Branches     14972     14957       -15     
=============================================
+ Hits         29751     29756        +5     
+ Misses        4791      4789        -2     
- Partials        42        44        +2
Files with missing lines Coverage Δ
Sources/Sentry/SentryOptions.m 98.761% <100.000%> (ø)
Sources/Swift/SentryExperimentalOptions.swift 81.818% <100.000%> (ø)

... and 12 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9aa0b44...ad32dbb. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 31, 2025 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1228.20 ms 1251.25 ms 23.05 ms
Size 23.75 KiB 842.68 KiB 818.94 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
e4cc043 1230.10 ms 1254.36 ms 24.26 ms
103d817 1211.55 ms 1213.35 ms 1.80 ms
983de17 1260.57 ms 1263.68 ms 3.11 ms
e070f8a 1236.47 ms 1250.50 ms 14.03 ms
afce5bf 1199.94 ms 1213.27 ms 13.33 ms
b9b0f0a 1257.22 ms 1262.57 ms 5.35 ms
60bfc91 1251.29 ms 1261.67 ms 10.39 ms
3fc6a6f 1241.96 ms 1254.13 ms 12.17 ms
6e625c5 1243.47 ms 1243.58 ms 0.11 ms
2deb275 1228.20 ms 1245.02 ms 16.82 ms

App size

Revision Plain With Sentry Diff
e4cc043 22.85 KiB 414.92 KiB 392.07 KiB
103d817 21.58 KiB 571.91 KiB 550.33 KiB
983de17 22.84 KiB 403.19 KiB 380.34 KiB
e070f8a 21.58 KiB 546.20 KiB 524.62 KiB
afce5bf 21.58 KiB 542.18 KiB 520.60 KiB
b9b0f0a 20.76 KiB 434.94 KiB 414.17 KiB
60bfc91 20.76 KiB 434.94 KiB 414.18 KiB
3fc6a6f 22.30 KiB 821.37 KiB 799.07 KiB
6e625c5 22.85 KiB 413.41 KiB 390.56 KiB
2deb275 21.58 KiB 681.72 KiB 660.14 KiB

Previous results on branch: experimentalOptionsOnlySwift

Startup times

Revision Plain With Sentry Diff
0bceca4 1220.57 ms 1243.64 ms 23.07 ms
0743827 1220.46 ms 1242.37 ms 21.91 ms
991861a 1229.98 ms 1251.08 ms 21.10 ms
ebc9d20 1235.54 ms 1246.39 ms 10.85 ms
a8d141f 1228.21 ms 1251.68 ms 23.47 ms
d0c8691 1230.51 ms 1243.77 ms 13.26 ms
66f760c 1211.69 ms 1232.22 ms 20.54 ms
fa2c9b4 1218.28 ms 1240.78 ms 22.50 ms

App size

Revision Plain With Sentry Diff
0bceca4 23.75 KiB 841.34 KiB 817.60 KiB
0743827 23.75 KiB 841.40 KiB 817.65 KiB
991861a 23.76 KiB 831.30 KiB 807.54 KiB
ebc9d20 23.76 KiB 837.47 KiB 813.71 KiB
a8d141f 23.76 KiB 836.52 KiB 812.76 KiB
d0c8691 23.76 KiB 831.30 KiB 807.54 KiB
66f760c 23.75 KiB 841.58 KiB 817.83 KiB
fa2c9b4 23.75 KiB 841.58 KiB 817.84 KiB

@noahsmartin noahsmartin force-pushed the experimentalOptionsOnlySwift branch from 365d42e to c08c7b4 Compare May 31, 2025 02:58
philipphofmann
philipphofmann approved these changes Jun 2, 2025
View reviewed changes
Copy link
Member

@philipphofmann philipphofmann left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's give this approach a shot and see how it goes.

I think we still should add a changelog entry mentioning this as an improvement, so we know for reference in the changelog when we changed this.

@noahsmartin noahsmartin force-pushed the experimentalOptionsOnlySwift branch 6 times, most recently from 294307a to e63030c Compare June 9, 2025 19:49
@noahsmartin noahsmartin force-pushed the experimentalOptionsOnlySwift branch from e63030c to ad32dbb Compare June 9, 2025 20:56
@noahsmartin noahsmartin merged commit 7e16c0c into main Jun 10, 2025
99 of 104 checks passed
@noahsmartin noahsmartin deleted the experimentalOptionsOnlySwift branch June 10, 2025 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipphofmann philipphofmann philipphofmann approved these changes

@armcknight armcknight Awaiting requested review from armcknight

@philprime philprime Awaiting requested review from philprime philprime is a code owner

@trevor-e trevor-e Awaiting requested review from trevor-e

@itaybre itaybre Awaiting requested review from itaybre itaybre is a code owner

@NicoHinderling NicoHinderling Awaiting requested review from NicoHinderling

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@noahsmartin @philipphofmann