Skip to content
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.

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

ref: access SentrySwizzleWrapper via existing SentryDependencyContainer property #3245

Merged

Conversation

armcknight
Copy link
Member

We already had a property on SentryDependencyContainer for this, but that still accessed SentrySwizzleWrapper.sharedInstance and we still also passed around the reference.

#skip-changelog

@codecov
Copy link

codecov bot commented Aug 19, 2023

Codecov Report

Merging #3245 (26cb0bd) into main (01a28a9) will increase coverage by 0.012%.
The diff coverage is 100.000%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3245       +/-   ##
=============================================
+ Coverage   89.112%   89.125%   +0.012%     
=============================================
  Files          502       502               
  Lines        54118     54135       +17     
  Branches     19429     19438        +9     
=============================================
+ Hits         48226     48248       +22     
+ Misses        5029      5026        -3     
+ Partials       863       861        -2     
Files Changed Coverage Δ
Sources/Sentry/SentrySwizzleWrapper.m 100.000% <ø> (ø)
...s/Sentry/SentryAutoBreadcrumbTrackingIntegration.m 100.000% <100.000%> (ø)
Sources/Sentry/SentryBreadcrumbTracker.m 63.953% <100.000%> (-1.019%) ⬇️
Sources/Sentry/SentryDependencyContainer.m 94.594% <100.000%> (ø)
Sources/Sentry/SentryUIEventTracker.m 100.000% <100.000%> (ø)
Sources/Sentry/SentryUIEventTrackingIntegration.m 100.000% <100.000%> (ø)
...SentryTests/Helper/SentrySwizzleWrapperTests.swift 87.500% <100.000%> (ø)
...SentryAutoBreadcrumbTrackingIntegrationTests.swift 97.590% <100.000%> (ø)
...ons/Breadcrumbs/SentryBreadcrumbTrackerTests.swift 100.000% <100.000%> (ø)
...egrations/UIEvents/SentryUIEventTrackerTests.swift 100.000% <100.000%> (ø)

... 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 01a28a9...26cb0bd. Read the comment docs.

@github-actions
Copy link

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1233.71 ms 1245.72 ms 12.01 ms
Size 22.85 KiB 405.01 KiB 382.16 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9ef729b 1228.79 ms 1245.36 ms 16.57 ms
11ccc16 1203.82 ms 1237.06 ms 33.24 ms
8c8654d 1214.43 ms 1223.88 ms 9.45 ms
d257eb9 1226.08 ms 1256.54 ms 30.46 ms
c319795 1220.06 ms 1248.50 ms 28.44 ms
aa27ac0 1214.02 ms 1227.42 ms 13.40 ms
c9724f9 1199.38 ms 1229.54 ms 30.16 ms
cd3bfeb 1226.61 ms 1226.96 ms 0.35 ms
c0b4b71 1218.16 ms 1251.28 ms 33.12 ms
1bbcb9c 1214.25 ms 1230.04 ms 15.79 ms

App size

Revision Plain With Sentry Diff
9ef729b 20.76 KiB 432.88 KiB 412.12 KiB
11ccc16 20.76 KiB 431.71 KiB 410.95 KiB
8c8654d 22.84 KiB 403.18 KiB 380.33 KiB
d257eb9 20.76 KiB 433.23 KiB 412.47 KiB
c319795 20.76 KiB 431.99 KiB 411.22 KiB
aa27ac0 20.76 KiB 432.21 KiB 411.45 KiB
c9724f9 20.76 KiB 427.66 KiB 406.90 KiB
cd3bfeb 20.76 KiB 425.77 KiB 405.01 KiB
c0b4b71 20.76 KiB 430.98 KiB 410.22 KiB
1bbcb9c 20.76 KiB 426.10 KiB 405.34 KiB

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

lgtm

@armcknight armcknight merged commit 9c7f314 into main Aug 29, 2023
66 checks passed
@armcknight armcknight deleted the armcknight/ref/centralize-SentrySwizzleWrapper-singleton branch August 29, 2023 17:01
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