Skip to content

Conversation

@noahsmartin
Copy link
Contributor

@noahsmartin noahsmartin commented Oct 28, 2025

I was hoping to remove SentrySerializable from the public API in V9, however attempts at doing so hit compiler issues in the cocoapods setup that I haven't been able to resolve yet: #6522

There are other higher priority changes for V9 so I suggest we just remove these preprocessor checks and focus on the other V9 changes, we can always come back to this if there's time later

Closes #6575

cursor[bot]

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.387%. Comparing base (005f255) to head (7c6f243).
⚠️ Report is 9 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #6574       +/-   ##
=============================================
+ Coverage   85.368%   85.387%   +0.018%     
=============================================
  Files          451       451               
  Lines        27428     27428               
  Branches     11936     11936               
=============================================
+ Hits         23415     23420        +5     
+ Misses        3967      3964        -3     
+ Partials        46        44        -2     
Files with missing lines Coverage Δ
...ces/Sentry/Profiling/SentryProfilerSerialization.m 64.435% <ø> (ø)
Sources/Sentry/SentryCrashIntegration.m 98.936% <ø> (ø)
Sources/Sentry/SentryCrashScopeObserver.m 94.029% <ø> (ø)
Sources/Sentry/SentryEnvelopeAttachmentHeader.m 100.000% <ø> (ø)
Sources/Sentry/SentryEvent.m 98.979% <ø> (ø)
Sources/Sentry/SentryException.m 100.000% <ø> (ø)
Sources/Sentry/SentryFileManagerHelper.m 88.116% <ø> (ø)
Sources/Sentry/SentryMechanism.m 100.000% <ø> (ø)
Sources/Sentry/SentryMechanismMeta.m 100.000% <ø> (ø)
Sources/Sentry/SentryScope.m 97.080% <ø> (ø)
... and 7 more

... and 10 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 005f255...7c6f243. Read the comment docs.

Copy link
Contributor

@itaybre itaybre left a comment

Choose a reason for hiding this comment

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

LGTM, but there are some imports that may be broken

@noahsmartin noahsmartin force-pushed the removeV9ChecksSerialize branch from 2110ae2 to 0d71332 Compare October 28, 2025 18:56
@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1224.04 ms 1266.73 ms 42.69 ms
Size 23.75 KiB 1.01 MiB 1016.13 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b9ceffb 1222.57 ms 1247.96 ms 25.39 ms
3133d0e 1237.86 ms 1262.87 ms 25.01 ms
9add417 1224.33 ms 1243.06 ms 18.73 ms
07f6759 1232.98 ms 1267.59 ms 34.61 ms
8ad303c 1220.02 ms 1231.79 ms 11.77 ms
c4b0360 1235.68 ms 1252.65 ms 16.97 ms
8fd192f 1202.10 ms 1220.19 ms 18.09 ms
42cfd79 1222.13 ms 1244.23 ms 22.10 ms
25f2d2c 1232.02 ms 1242.78 ms 10.76 ms
5712478 1220.10 ms 1239.69 ms 19.59 ms

App size

Revision Plain With Sentry Diff
b9ceffb 23.75 KiB 969.07 KiB 945.32 KiB
3133d0e 23.74 KiB 976.79 KiB 953.04 KiB
9add417 23.75 KiB 908.40 KiB 884.65 KiB
07f6759 23.75 KiB 997.26 KiB 973.52 KiB
8ad303c 23.75 KiB 879.24 KiB 855.49 KiB
c4b0360 23.75 KiB 946.69 KiB 922.94 KiB
8fd192f 23.74 KiB 872.75 KiB 849.01 KiB
42cfd79 23.75 KiB 880.20 KiB 856.45 KiB
25f2d2c 23.75 KiB 866.69 KiB 842.94 KiB
5712478 23.75 KiB 969.28 KiB 945.54 KiB

Previous results on branch: removeV9ChecksSerialize

Startup times

Revision Plain With Sentry Diff
c731172 1229.07 ms 1248.45 ms 19.38 ms
3e26066 1214.74 ms 1254.63 ms 39.88 ms

App size

Revision Plain With Sentry Diff
c731172 23.75 KiB 1.01 MiB 1016.13 KiB
3e26066 23.75 KiB 1.01 MiB 1016.13 KiB

@noahsmartin noahsmartin force-pushed the removeV9ChecksSerialize branch from 0d71332 to f4a5aad Compare October 28, 2025 19:31
@noahsmartin noahsmartin force-pushed the removeV9ChecksSerialize branch from f4a5aad to 7c6f243 Compare October 28, 2025 20:00
@github-actions
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 merged commit e648312 into main Oct 29, 2025
184 of 191 checks passed
@noahsmartin noahsmartin deleted the removeV9ChecksSerialize branch October 29, 2025 12:48
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.

ref: Remove SDK_V9 checks around SentrySerializable

3 participants