-
-
Notifications
You must be signed in to change notification settings - Fork 348
test: shared sdk wrapper other platforms #5085
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
base: armcknight/test/share-sdk-wrapper
Are you sure you want to change the base?
test: shared sdk wrapper other platforms #5085
Conversation
78e25e9
to
3112778
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple things I noticed as I went through this that I think are beneficial right off the bat
|
||
options.enableAutoSessionTracking = !SentrySDKOverrides.Performance.disableSessionTracking.boolValue | ||
if let sessionTrackingIntervalMillis = env["--io.sentry.sessionTrackingIntervalMillis"] { | ||
options.sessionTrackingIntervalMillis = UInt((sessionTrackingIntervalMillis as NSString).integerValue) | ||
} else { | ||
options.sessionTrackingIntervalMillis = 5_000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wasn't set for many of the sample apps as the default
#endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) || os(visionOS) | ||
|
||
#if os(macOS) | ||
options.enableUncaughtNSExceptionReporting = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was only set for macOS-SwiftUI but not macOS-Swift
return scope | ||
} | ||
|
||
options.experimental.enableFileManagerSwizzling = !args.contains("--disable-filemanager-swizzling") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This launch arg introduced in #4634 wasn't actually configured in any of our shared schemas, I shared it in the upstream PR at https://github.com/getsentry/sentry-cocoa/pull/5076/files#diff-384a7a6c421086af40fa69500d57db0f5122e16c6c2753bf6bbf60447e2a3c06R105 so now all sample apps can use it
3112778
to
783f991
Compare
22f774b
to
4933865
Compare
…/shared-sdk-wrapper-other-platforms
…/shared-sdk-wrapper-other-platforms
…/shared-sdk-wrapper-other-platforms
Following on #5076 and #5077, switch the remaining sample apps implemented in Swift on other platforms to use the shared SDK wrapper.
#skip-changelog