-
-
Notifications
You must be signed in to change notification settings - Fork 343
ref(ios): Extract Cocoa SDK init into standalone file #4442
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(ios): Extract Cocoa SDK init into standalone file #4442
Conversation
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
650ad89 | 448.65 ms | 437.68 ms | -10.97 ms |
aca58c3 | 466.10 ms | 450.37 ms | -15.73 ms |
6326ab7 | 445.10 ms | 434.72 ms | -10.39 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
650ad89 | 17.75 MiB | 20.11 MiB | 2.36 MiB |
aca58c3 | 17.75 MiB | 20.11 MiB | 2.36 MiB |
6326ab7 | 17.75 MiB | 20.11 MiB | 2.37 MiB |
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
6326ab7+dirty | 390.48 ms | 445.07 ms | 54.59 ms |
650ad89+dirty | 317.38 ms | 328.64 ms | 11.26 ms |
aca58c3+dirty | 390.15 ms | 460.67 ms | 70.52 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
6326ab7+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
650ad89+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
aca58c3+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
920767a
to
ac49981
Compare
ac49981
to
4897aa3
Compare
iOS (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
650ad89+dirty | 1248.86 ms | 1252.69 ms | 3.84 ms |
aca58c3+dirty | 1227.92 ms | 1243.36 ms | 15.44 ms |
6326ab7+dirty | 1211.35 ms | 1212.63 ms | 1.27 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
650ad89+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
aca58c3+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
6326ab7+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
iOS (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
650ad89+dirty | 1231.02 ms | 1239.31 ms | 8.29 ms |
aca58c3+dirty | 1233.37 ms | 1239.63 ms | 6.27 ms |
6326ab7+dirty | 1228.06 ms | 1224.84 ms | -3.23 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
650ad89+dirty | 2.63 MiB | 3.68 MiB | 1.05 MiB |
aca58c3+dirty | 2.63 MiB | 3.68 MiB | 1.05 MiB |
6326ab7+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
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.
The refactoring LGTM and the sample app worked as expected for me 🚀
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.
I didn't run the tests locally, but the cocoa code looks fine to me.
@ @philipphofmann @brustolin I would appreciate a second opinion
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.
LGTM, for moving stuff around.
📢 Type of change
📜 Description
This PR doesn't change any of the initialization logic, only moves the code from the main module file to a standalone class for future changes.
💡 Motivation and Context
startWithConfigureOptions
for Apple platforms #4444💚 How did you test it?
sample app
📝 Checklist
sendDefaultPII
is enabled