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

iOS: native support #254

Merged
merged 57 commits into from
Aug 6, 2021
Merged

iOS: native support #254

merged 57 commits into from
Aug 6, 2021

Conversation

bitsandfoxes
Copy link
Contributor

@bitsandfoxes bitsandfoxes commented Jul 13, 2021

First working iteration on our iOS Native Support:

  • Restore Cocoa submodule if missing
  • Build/Clean Cocoa SDK targets (dotnet msbuild /t:BuildCocoaSDK \ CleanCocoaSDK)
  • Copy Sentry.framework into package-dev
  • Hook into Unity Xcode generation during build post process and add the Sentry SDK
  • Generate native options dictionary and fill it with the appropriate values read from ScriptableSentryOptions
  • Hook into main and initialize the SDK

@bitsandfoxes bitsandfoxes added the Feature New feature or request label Jul 13, 2021
@bitsandfoxes bitsandfoxes self-assigned this Jul 13, 2021
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

nice tests

src/Sentry.Unity.Editor.iOS/MainModifier.cs Outdated Show resolved Hide resolved
Directory.Build.targets Outdated Show resolved Hide resolved
scripts/build-cocoa-sdk.sh Outdated Show resolved Hide resolved
src/Sentry.Unity.Editor.iOS/BuildPostProcess.cs Outdated Show resolved Hide resolved
src/Sentry.Unity.Editor.iOS/NativeMain.cs Outdated Show resolved Hide resolved
// IMPORTANT: Changes to this file will be lost!
// This file is generated during the Xcode project creation.

// TODO: make pretty with link to docs
Copy link
Member

Choose a reason for hiding this comment

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

👍

src/Sentry.Unity.Editor.iOS/SentryXcodeProject.cs Outdated Show resolved Hide resolved
src/Sentry.Unity.Editor.iOS/SentryXcodeProject.cs Outdated Show resolved Hide resolved
src/Sentry.Unity.Editor.iOS/SentryXcodeProject.cs Outdated Show resolved Hide resolved
src/Sentry.Unity.Editor.iOS/SentryXcodeProject.cs Outdated Show resolved Hide resolved
@bruno-garcia
Copy link
Member

Good stuff! Left suggestions for some minor improvements but this is great!

@bruno-garcia bruno-garcia reopened this Aug 3, 2021
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

🚀

Directory.Build.targets Outdated Show resolved Hide resolved
Directory.Build.targets Show resolved Hide resolved
<ShutdownTimeout>k__BackingField: 2000
<MaxQueueItems>k__BackingField: 30
<IOSNativeSupportEnabled>k__BackingField: 1
Copy link
Member

Choose a reason for hiding this comment

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

We need to consider the upgrade path. Where this field doesn't exist and will default to 0.
Perhaps have it nullable so we can tell if the user opt-ed out or if it's just an upgrading scriptable objet

src/Sentry.Unity.Editor.iOS/BuildPostProcess.cs Outdated Show resolved Hide resolved
test/Sentry.Unity.Editor.iOS.Tests/NativeMainTests.cs Outdated Show resolved Hide resolved
test/Sentry.Unity.Editor.iOS.Tests/NativeMainTests.cs Outdated Show resolved Hide resolved
test/Sentry.Unity.Editor.iOS.Tests/NativeMainTests.cs Outdated Show resolved Hide resolved
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Awesome stuff! I left some minor notes but we could merge and address other things in follow up PRs

src/Sentry.Unity.Editor.iOS/BuildPostProcess.cs Outdated Show resolved Hide resolved
src/Sentry.Unity.Editor.iOS/NativeOptions.cs Outdated Show resolved Hide resolved
src/Sentry.Unity.Editor.iOS/SentryXcodeProject.cs Outdated Show resolved Hide resolved
src/Sentry.Unity.Editor.iOS/SentryXcodeProject.cs Outdated Show resolved Hide resolved
src/Sentry.Unity.Editor.iOS/SentryXcodeProject.cs Outdated Show resolved Hide resolved
src/Sentry.Unity/SentryUnityOptionsExtensions.cs Outdated Show resolved Hide resolved
test/Sentry.Unity.Editor.iOS.Tests/NativeOptionsTests.cs Outdated Show resolved Hide resolved
@bitsandfoxes bitsandfoxes merged commit 3c129a3 into main Aug 6, 2021
@bitsandfoxes bitsandfoxes deleted the feat/ios_native branch August 6, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants