Skip to content

Conversation

noahsmartin
Copy link
Contributor

Convert this file to Swift

#skip-changelog

Copy link

codecov bot commented Jun 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.756%. Comparing base (d1c0538) to head (a8889ea).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5481       +/-   ##
=============================================
- Coverage   86.760%   86.756%   -0.005%     
=============================================
  Files          424       424               
  Lines        36723     36727        +4     
  Branches     17365     17356        -9     
=============================================
+ Hits         31861     31863        +2     
- Misses        4815      4819        +4     
+ Partials        47        45        -2     
Files with missing lines Coverage Δ
Sources/Sentry/SentryClient.m 98.940% <100.000%> (+0.004%) ⬆️
Sources/Sentry/SentryEnvelope.m 88.839% <100.000%> (+0.151%) ⬆️
Sources/Sentry/SentryHub.m 98.086% <ø> (ø)
Sources/Sentry/SentrySerialization.m 98.905% <ø> (ø)
...ources/Swift/Core/Helper/SentryExtraPackages.swift 95.238% <100.000%> (ø)
Sources/Swift/Helper/SentrySdkInfo.swift 100.000% <100.000%> (ø)

... and 4 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 d1c0538...a8889ea. Read the comment docs.

Copy link
Contributor

github-actions bot commented Jun 24, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1234.08 ms 1251.78 ms 17.70 ms
Size 23.75 KiB 930.68 KiB 906.93 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
605fa27 1226.31 ms 1251.35 ms 25.05 ms
cc02c0d 1234.69 ms 1247.15 ms 12.46 ms
7f2f69c 1237.61 ms 1266.96 ms 29.35 ms
4d264fa 1223.48 ms 1246.91 ms 23.44 ms
409a607 1229.57 ms 1251.45 ms 21.88 ms
80a5166 1224.49 ms 1251.29 ms 26.80 ms
029a804 1219.65 ms 1241.96 ms 22.30 ms
04ff3ec 1220.71 ms 1253.86 ms 33.15 ms
aadbffe 1228.73 ms 1251.59 ms 22.86 ms
73c9712 1238.57 ms 1260.38 ms 21.80 ms

App size

Revision Plain With Sentry Diff
605fa27 23.75 KiB 908.03 KiB 884.28 KiB
cc02c0d 23.75 KiB 912.37 KiB 888.62 KiB
7f2f69c 23.75 KiB 913.38 KiB 889.63 KiB
4d264fa 23.74 KiB 874.07 KiB 850.33 KiB
409a607 23.74 KiB 874.08 KiB 850.33 KiB
80a5166 23.75 KiB 904.53 KiB 880.78 KiB
029a804 23.75 KiB 928.11 KiB 904.36 KiB
04ff3ec 23.75 KiB 880.26 KiB 856.52 KiB
aadbffe 23.75 KiB 912.77 KiB 889.02 KiB
73c9712 23.75 KiB 908.01 KiB 884.26 KiB

Previous results on branch: convertSentrySdkInfo

Startup times

Revision Plain With Sentry Diff
ab9fe41 1202.86 ms 1256.56 ms 53.71 ms
1796542 1232.46 ms 1257.69 ms 25.24 ms
87a7915 1227.19 ms 1250.33 ms 23.14 ms
e2ca600 1225.83 ms 1248.22 ms 22.39 ms
2106b14 1240.76 ms 1263.91 ms 23.16 ms
c7ba9de 1233.92 ms 1249.49 ms 15.57 ms
e48f485 1211.18 ms 1233.71 ms 22.53 ms

App size

Revision Plain With Sentry Diff
ab9fe41 23.75 KiB 857.88 KiB 834.13 KiB
1796542 23.75 KiB 930.68 KiB 906.94 KiB
87a7915 23.75 KiB 930.68 KiB 906.94 KiB
e2ca600 23.75 KiB 930.68 KiB 906.93 KiB
2106b14 23.75 KiB 927.79 KiB 904.04 KiB
c7ba9de 23.75 KiB 930.68 KiB 906.93 KiB
e48f485 23.75 KiB 857.68 KiB 833.93 KiB

@noahsmartin noahsmartin force-pushed the convertSentrySdkInfo branch 2 times, most recently from 30e5a2c to 443be4f Compare June 25, 2025 02:13
@noahsmartin noahsmartin marked this pull request as draft June 25, 2025 02:49
@noahsmartin noahsmartin force-pushed the convertSentrySdkInfo branch 4 times, most recently from 18a5ec2 to 579b1e8 Compare August 15, 2025 17:54
@noahsmartin noahsmartin force-pushed the convertSentrySdkInfo branch 6 times, most recently from 4529ba2 to e7c9fb6 Compare August 28, 2025 20:35
@noahsmartin noahsmartin force-pushed the convertSentrySdkInfo branch from e7c9fb6 to a8889ea Compare August 28, 2025 20:52
Copy link
Contributor

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentrySerialization.m

@noahsmartin noahsmartin marked this pull request as ready for review August 28, 2025 21:19
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 two minor comments

@@ -0,0 +1,69 @@
import XCTest

func compareEnvelopes(_ expectedEnvelope: Data?, _ actualEnvelope: Data?, message: String) throws {
Copy link
Member

Choose a reason for hiding this comment

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

l: Could be added as an extension of XCTest so it's limited in scope

import Foundation

extension Data {
func backwardCompatibleSplit(separator: Data) -> [Data] {
Copy link
Member

Choose a reason for hiding this comment

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

m: Please add documentation and inline-comments to this method so we know the purpose of it, and also know at which point in time we can get rid of the backwards compatilibity

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.

2 participants