Skip to content

Conversation

@noahsmartin
Copy link
Contributor

@noahsmartin noahsmartin commented Jun 27, 2025

This makes SentryUserFeedbackIntegrationDriver's init public, another step towards #5371

This function used SentryScreenshot, which was internal in ObjC so couldn't be added to an SPI function. I converted it to Swift and marked it SPI so it could be used as a parameter to an SPI function

Checked this in the iOS-swift sample app by sending test feedback, the issue with associated screenshot is found here:
https://sentry-sdks.sentry.io/issues/2219152762/events/5b005262adcf449d9d0f26bda2af517d/?project=5428557&referrer=event-or-group-header

#skip-changelog

@codecov
Copy link

codecov bot commented Jun 27, 2025

Codecov Report

Attention: Patch coverage is 82.75862% with 15 lines in your changes missing coverage. Please review.

Project coverage is 86.245%. Comparing base (61414e8) to head (a6996ed).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
Sources/Swift/Tools/SentryScreenshot.swift 82.500% 14 Missing ⚠️
...Feedback/SentryUserFeedbackIntegrationDriver.swift 0.000% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5516       +/-   ##
=============================================
+ Coverage   86.165%   86.245%   +0.080%     
=============================================
  Files          403       404        +1     
  Lines        34905     34935       +30     
  Branches     14972     15160      +188     
=============================================
+ Hits         30076     30130       +54     
+ Misses        4786      4762       -24     
  Partials        43        43               
Files with missing lines Coverage Δ
Sources/Sentry/PrivateSentrySDKOnly.m 24.019% <ø> (ø)
Sources/Sentry/SentryDependencyContainer.m 88.372% <ø> (ø)
...rces/Sentry/SentryDependencyContainerSwiftHelper.m 100.000% <100.000%> (ø)
Sources/Sentry/SentryScreenshotIntegration.m 92.156% <ø> (ø)
Sources/Sentry/SentryUserFeedbackIntegration.m 26.315% <ø> (ø)
...Feedback/SentryUserFeedbackIntegrationDriver.swift 0.000% <0.000%> (ø)
Sources/Swift/Tools/SentryScreenshot.swift 82.500% <82.500%> (ø)

... and 14 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 61414e8...a6996ed. 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
Copy link
Contributor

github-actions bot commented Jun 27, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1230.61 ms 1255.22 ms 24.61 ms
Size 23.75 KiB 872.67 KiB 848.92 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
35c962f 1207.61 ms 1235.90 ms 28.29 ms
d38165b 1211.41 ms 1242.49 ms 31.08 ms
5cfc768 1220.74 ms 1245.06 ms 24.32 ms
8047b99 1226.37 ms 1246.63 ms 20.26 ms
b13e93a 1236.24 ms 1247.33 ms 11.08 ms
db9572a 1200.27 ms 1234.80 ms 34.53 ms
8ea5293 1242.70 ms 1262.25 ms 19.55 ms
018037b 1209.31 ms 1228.33 ms 19.03 ms
f97a070 1218.88 ms 1253.12 ms 34.24 ms
acac774 1217.76 ms 1253.29 ms 35.52 ms

App size

Revision Plain With Sentry Diff
35c962f 23.75 KiB 854.77 KiB 831.02 KiB
d38165b 23.75 KiB 855.37 KiB 831.62 KiB
5cfc768 23.75 KiB 850.73 KiB 826.98 KiB
8047b99 23.75 KiB 855.37 KiB 831.62 KiB
b13e93a 23.75 KiB 855.37 KiB 831.62 KiB
db9572a 23.75 KiB 858.69 KiB 834.93 KiB
8ea5293 23.75 KiB 852.24 KiB 828.49 KiB
018037b 23.75 KiB 867.16 KiB 843.41 KiB
f97a070 23.75 KiB 858.68 KiB 834.93 KiB
acac774 23.75 KiB 866.51 KiB 842.76 KiB

Previous results on branch: sentryUserFeedbackIntergationDriverSPI

Startup times

Revision Plain With Sentry Diff
945c223 1232.13 ms 1258.02 ms 25.89 ms

App size

Revision Plain With Sentry Diff
945c223 23.75 KiB 871.06 KiB 847.31 KiB

@noahsmartin noahsmartin force-pushed the sentryUserFeedbackIntergationDriverSPI branch 7 times, most recently from 6fd14b1 to 6889315 Compare June 27, 2025 19:16
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM, when doing some manual test runs with the iOS-Swift sample app and validating that screenshots still work, because I don't think we have an e2e test coverage for the screenshots feature. Please post the URLs of the events, including some screenshots, to show that the feature still works.

@noahsmartin noahsmartin force-pushed the sentryUserFeedbackIntergationDriverSPI branch from be4c840 to a6996ed Compare June 30, 2025 16:34
@noahsmartin noahsmartin merged commit a2a3bfb into main Jun 30, 2025
139 of 143 checks passed
@noahsmartin noahsmartin deleted the sentryUserFeedbackIntergationDriverSPI branch June 30, 2025 17:34
philipsawyerdd added a commit to justin-doordash/sentry-cocoa that referenced this pull request Sep 25, 2025
philipsawyerdd added a commit to justin-doordash/sentry-cocoa that referenced this pull request Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants