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

SPM Support #2280

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

SPM Support #2280

wants to merge 30 commits into from

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Sep 10, 2024

📜 Description

Migration: https://docs.flutter.dev/packages-and-plugins/swift-package-manager/for-plugin-authors#how-to-add-swift-package-manager-support-to-an-existing-flutter-plugin

Enable SPM: https://docs.flutter.dev/packages-and-plugins/swift-package-manager/for-plugin-authors#how-to-turn-on-swift-package-manager

  • Setup iOS project for SPM
  • Setup macOS project for SPM
  • Update plugin to import Hybrid SDK for Pods & SPM

💡 Motivation and Context

Closes #2250

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

Copy link
Contributor

github-actions bot commented Sep 10, 2024

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 5f38dda

Copy link
Contributor

github-actions bot commented Sep 10, 2024

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 432.24 ms 529.78 ms 97.54 ms
Size 6.46 MiB 7.48 MiB 1.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
1cdcacf 389.94 ms 463.53 ms 73.59 ms
be08ed1 361.37 ms 414.84 ms 53.47 ms
3ad66e4 406.90 ms 481.48 ms 74.58 ms
ad69abc 297.35 ms 385.89 ms 88.54 ms
ef6466d 373.96 ms 443.17 ms 69.21 ms
e893df5 310.60 ms 380.58 ms 69.98 ms
f25f207 341.40 ms 427.73 ms 86.34 ms
61e71ec 343.94 ms 410.59 ms 66.66 ms
1131914 317.90 ms 361.58 ms 43.69 ms
fe4aa56 356.06 ms 428.67 ms 72.61 ms

App size

Revision Plain With Sentry Diff
1cdcacf 6.33 MiB 7.26 MiB 949.77 KiB
be08ed1 6.33 MiB 7.26 MiB 946.42 KiB
3ad66e4 6.33 MiB 7.26 MiB 947.04 KiB
ad69abc 6.06 MiB 7.09 MiB 1.03 MiB
ef6466d 6.34 MiB 7.28 MiB 967.79 KiB
e893df5 6.06 MiB 7.09 MiB 1.03 MiB
f25f207 6.33 MiB 7.26 MiB 943.41 KiB
61e71ec 6.34 MiB 7.28 MiB 966.26 KiB
1131914 5.94 MiB 6.96 MiB 1.02 MiB
fe4aa56 6.06 MiB 7.10 MiB 1.04 MiB

Previous results on branch: feat/spm

Startup times

Revision Plain With Sentry Diff
cf145b9 449.38 ms 538.56 ms 89.18 ms
acc2ff0 470.52 ms 510.29 ms 39.77 ms
8adbf8e 481.79 ms 530.56 ms 48.77 ms
160e5a5 460.87 ms 509.56 ms 48.69 ms
a4e2e38 459.34 ms 506.62 ms 47.28 ms

App size

Revision Plain With Sentry Diff
cf145b9 6.46 MiB 7.48 MiB 1.01 MiB
acc2ff0 6.49 MiB 7.57 MiB 1.08 MiB
8adbf8e 6.49 MiB 7.55 MiB 1.07 MiB
160e5a5 6.49 MiB 7.57 MiB 1.08 MiB
a4e2e38 6.49 MiB 7.57 MiB 1.08 MiB

Copy link
Contributor

github-actions bot commented Sep 10, 2024

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1259.00 ms 1276.92 ms 17.92 ms
Size 8.42 MiB 9.86 MiB 1.44 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9fe67d5 1242.33 ms 1268.77 ms 26.44 ms
a775d3f 1241.29 ms 1241.53 ms 0.24 ms
103eb14 1254.11 ms 1280.65 ms 26.55 ms
754cdbe 1261.67 ms 1280.88 ms 19.20 ms
408bdab 1239.63 ms 1257.61 ms 17.98 ms
fbf42af 1253.76 ms 1269.51 ms 15.76 ms
c73ab67 1267.73 ms 1279.36 ms 11.63 ms
99704d2 1241.74 ms 1258.00 ms 16.26 ms
bd1b990 1209.43 ms 1226.96 ms 17.53 ms
4c13d97 1265.41 ms 1287.95 ms 22.55 ms

App size

Revision Plain With Sentry Diff
9fe67d5 8.32 MiB 9.50 MiB 1.18 MiB
a775d3f 8.28 MiB 9.34 MiB 1.06 MiB
103eb14 8.38 MiB 9.71 MiB 1.34 MiB
754cdbe 8.29 MiB 9.37 MiB 1.08 MiB
408bdab 8.28 MiB 9.34 MiB 1.06 MiB
fbf42af 8.16 MiB 9.17 MiB 1.01 MiB
c73ab67 8.29 MiB 9.36 MiB 1.07 MiB
99704d2 8.42 MiB 9.87 MiB 1.44 MiB
bd1b990 8.32 MiB 9.38 MiB 1.06 MiB
4c13d97 8.38 MiB 9.78 MiB 1.40 MiB

Previous results on branch: feat/spm

Startup times

Revision Plain With Sentry Diff
acc2ff0 1223.71 ms 1233.08 ms 9.37 ms
cf145b9 1253.06 ms 1261.96 ms 8.90 ms
8adbf8e 1258.88 ms 1281.85 ms 22.97 ms
a4e2e38 1234.78 ms 1256.79 ms 22.01 ms
160e5a5 1250.08 ms 1277.88 ms 27.79 ms

App size

Revision Plain With Sentry Diff
acc2ff0 8.38 MiB 9.75 MiB 1.37 MiB
cf145b9 8.42 MiB 9.86 MiB 1.43 MiB
8adbf8e 8.38 MiB 9.73 MiB 1.36 MiB
a4e2e38 8.38 MiB 9.75 MiB 1.37 MiB
160e5a5 8.38 MiB 9.75 MiB 1.37 MiB

# Conflicts:
#	flutter/.gitignore
#	flutter/ios/sentry_flutter.podspec
uses locally build Swift.framwork for sim only
Copy link
Contributor

github-actions bot commented Nov 5, 2024

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/ios/Classes/SentryFlutterPluginApple.swift

Copy link

codecov bot commented Nov 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.59%. Comparing base (dd25e43) to head (5f38dda).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2280      +/-   ##
==========================================
+ Coverage   86.96%   91.59%   +4.62%     
==========================================
  Files         263       88     -175     
  Lines        9378     3081    -6297     
==========================================
- Hits         8156     2822    -5334     
+ Misses       1222      259     -963     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Nov 6, 2024

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/ios/Classes/SentryFlutterPluginApple.swift

Copy link
Contributor

github-actions bot commented Nov 6, 2024

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/ios/Classes/SentryFlutterPluginApple.swift

Copy link
Contributor

github-actions bot commented Nov 6, 2024

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/ios/Classes/SentryFlutterPluginApple.swift

Copy link
Contributor

github-actions bot commented Nov 6, 2024

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/ios/Classes/SentryFlutterPluginApple.swift

# Conflicts:
#	flutter/ios/sentry_flutter.podspec
Copy link
Contributor

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/ios/Classes/SentryFlutterPluginApple.swift

Copy link
Contributor

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/ios/Classes/SentryFlutterPluginApple.swift

# Conflicts:
#	flutter/ios/Classes/SentryFlutterPlugin.h
# Conflicts:
#	flutter/ios/sentry_flutter/Sources/sentry_flutter/SentryFlutterPlugin.swift
@denrase
Copy link
Collaborator Author

denrase commented Dec 9, 2024

@buenaflor Works with SPM, but we still need to wait until sentry-cocoa is out of beta.

Bildschirmfoto 2024-12-09 um 16 16 14

@denrase denrase marked this pull request as ready for review December 17, 2024 13:19
@denrase
Copy link
Collaborator Author

denrase commented Dec 17, 2024

@buenaflor sentry-cocoa with SPM support is now release. [Flutter 3.27] with SPM support also has landed.

@denrase denrase requested a review from buenaflor December 17, 2024 13:20
@buenaflor buenaflor removed the Blocked label Dec 17, 2024
Copy link
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

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

looks good!

do we need to do anything else when rolling out a release when this is merged or should it work automatically?

# Conflicts:
#	flutter/ios/sentry_flutter.podspec
@denrase denrase requested a review from buenaflor December 19, 2024 10:49
@denrase denrase enabled auto-merge (squash) December 19, 2024 10:50
Copy link
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

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

great! for me it's approved but won't approve it now because there's a risk of this breaking something in the release pipeline and with x-mas coming up its a bit risky :D

so wdyt if we merge it when holidays are over? @denrase

@denrase
Copy link
Collaborator Author

denrase commented Dec 19, 2024

@buenaflor Sure, don't want to mess with releases before the holidays. :) Tested it on a new project, works without CocoaPods.

Bildschirmfoto 2024-12-19 um 14 43 07

\cc @ueman 👀

@denrase denrase disabled auto-merge December 19, 2024 13:46
@ueman
Copy link
Collaborator

ueman commented Dec 19, 2024

I'm currently unable to test this. I'm right between jobs at the moment, so I don't have access to a code base which uses Sentry. (No clue whether the new one uses Sentry 😐) But it looks similar to a SPM migration in one of my packages and that didn't get any bug reports yet.

@denrase
Copy link
Collaborator Author

denrase commented Dec 30, 2024

@ueman Thanks for the feedback and congrats on the new job! :)

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.

Swift Package Manger Support
3 participants