Skip to content

ref: Convert SentryNSURLRequest to Swift #5457

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 1 commit into from
Jun 23, 2025
Merged

ref: Convert SentryNSURLRequest to Swift #5457

merged 1 commit into from
Jun 23, 2025

Conversation

noahsmartin
Copy link
Contributor

📜 Description

Converts SentryNSURLRequest to Swift

💚 How did you test it?

Build + existing tests

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

@noahsmartin noahsmartin force-pushed the convertSentryUrlRequest branch from 50cdd68 to 462b673 Compare June 21, 2025 15:53
@codecov Codecov
Copy link

codecov bot commented Jun 21, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 95.65217% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.207%. Comparing base (95cfd20) to head (8e2453b).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
Sources/Swift/Tools/SentryURLRequestFactory.swift 95.454% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5457       +/-   ##
=============================================
- Coverage   86.221%   86.207%   -0.015%     
=============================================
  Files          399       399               
  Lines        34647     34648        +1     
  Branches     14983     14991        +8     
=============================================
- Hits         29873     29869        -4     
- Misses        4726      4736       +10     
+ Partials        48        43        -5
Files with missing lines Coverage Δ
Sources/Sentry/SentryHttpTransport.m 98.161% <ø> (ø)
Sources/Sentry/SentryNSDataUtils.m 97.619% <ø> (ø)
Sources/Sentry/SentryNSURLRequestBuilder.m 100.000% <100.000%> (ø)
Sources/Sentry/SentrySpotlightTransport.m 79.310% <ø> (ø)
Sources/Swift/Tools/SentryURLRequestFactory.swift 95.454% <95.454%> (ø)

... and 5 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 95cfd20...8e2453b. 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 Jun 21, 2025 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1226.60 ms 1249.67 ms 23.07 ms
Size 23.75 KiB 849.23 KiB 825.48 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9faf217 1268.86 ms 1274.82 ms 5.96 ms
b5bcbab 1222.24 ms 1243.20 ms 20.96 ms
b1b7d72 1246.31 ms 1261.00 ms 14.69 ms
83acf3e 1240.06 ms 1261.38 ms 21.31 ms
8ef07fb 1231.45 ms 1243.98 ms 12.53 ms
a9103fe 1221.49 ms 1243.33 ms 21.84 ms
7f691b5 1233.94 ms 1243.80 ms 9.86 ms
81c2187 1224.20 ms 1252.68 ms 28.48 ms
676e429 1225.76 ms 1235.49 ms 9.73 ms
7054676 1229.46 ms 1246.40 ms 16.94 ms

App size

Revision Plain With Sentry Diff
9faf217 20.76 KiB 419.70 KiB 398.94 KiB
b5bcbab 23.75 KiB 846.99 KiB 823.24 KiB
b1b7d72 20.76 KiB 436.33 KiB 415.57 KiB
83acf3e 21.58 KiB 539.87 KiB 518.29 KiB
8ef07fb 21.58 KiB 707.42 KiB 685.84 KiB
a9103fe 20.76 KiB 426.95 KiB 406.19 KiB
7f691b5 20.76 KiB 420.55 KiB 399.79 KiB
81c2187 21.58 KiB 736.53 KiB 714.94 KiB
676e429 21.58 KiB 614.73 KiB 593.15 KiB
7054676 21.58 KiB 542.28 KiB 520.70 KiB

Previous results on branch: convertSentryUrlRequest

Startup times

Revision Plain With Sentry Diff
dc1f35d 1225.94 ms 1242.09 ms 16.15 ms

App size

Revision Plain With Sentry Diff
dc1f35d 23.75 KiB 849.20 KiB 825.46 KiB

@noahsmartin noahsmartin force-pushed the convertSentryUrlRequest branch from 462b673 to 8e2453b Compare June 23, 2025 03:35
philprime
philprime approved these changes Jun 23, 2025
View reviewed changes
Copy link
Member

@philprime philprime left a comment

Choose a reason for hiding this comment

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

LGTM, left a couple of comments.

@noahsmartin noahsmartin merged commit 16b739d into main Jun 23, 2025
143 of 148 checks passed
@noahsmartin noahsmartin deleted the convertSentryUrlRequest branch June 23, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philprime philprime philprime approved these changes

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

@armcknight armcknight Awaiting requested review from armcknight

@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 @philprime