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

VendorId should be a String #1112

Merged
merged 10 commits into from
Nov 16, 2022
Merged

VendorId should be a String #1112

merged 10 commits into from
Nov 16, 2022

Conversation

marandaneto
Copy link
Contributor

@marandaneto marandaneto commented Nov 7, 2022

📜 Description

💡 Motivation and Context

getsentry/team-mobile#67

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

It's technically a breaking change but nobody should be using it directly.
Also its a bug fixing since the previous type does not work.

🔮 Next steps

@marandaneto
Copy link
Contributor Author

@romtsn or @markushi can we fix this on Android? otherwise the serialization would fail on Android anyway.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 296.94 ms 351.56 ms 54.62 ms
Size 5.94 MiB 6.95 MiB 1.01 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
613760b 373.42 ms 399.33 ms 25.92 ms
559d28f 302.35 ms 339.53 ms 37.18 ms
9f9f94f 331.04 ms 368.92 ms 37.88 ms
2f8f173 323.31 ms 373.29 ms 49.97 ms
af2d175 279.08 ms 312.37 ms 33.29 ms
3e5ee37 317.56 ms 366.84 ms 49.28 ms
48e79fd 354.22 ms 391.46 ms 37.24 ms
7f2b01d 304.94 ms 345.71 ms 40.78 ms
f922f8f 332.31 ms 374.67 ms 42.37 ms
a49594a 282.27 ms 344.84 ms 62.57 ms

App size

Revision Plain With Sentry Diff
613760b 5.94 MiB 6.92 MiB 1005.98 KiB
559d28f 5.94 MiB 6.92 MiB 1001.70 KiB
9f9f94f 5.94 MiB 6.95 MiB 1.01 MiB
2f8f173 5.94 MiB 6.95 MiB 1.01 MiB
af2d175 5.94 MiB 6.92 MiB 1001.83 KiB
3e5ee37 5.94 MiB 6.92 MiB 1001.19 KiB
48e79fd 5.94 MiB 6.95 MiB 1.01 MiB
7f2b01d 5.94 MiB 6.95 MiB 1.01 MiB
f922f8f 5.94 MiB 6.95 MiB 1.01 MiB
a49594a 5.94 MiB 6.95 MiB 1.01 MiB

@krystofwoldrich
Copy link
Member

I guess this need to wait for Android SDK.

@marandaneto
Copy link
Contributor Author

I guess this need to wait for Android SDK.

its in the making, no need to block the PR tho.

@marandaneto
Copy link
Contributor Author

@markushi can we cut a release on Android so we can unblock this?

@markushi
Copy link
Member

@marandaneto the required sentry-java changes will be shipped with 6.7.1, which is about to be published.

@marandaneto marandaneto enabled auto-merge (squash) November 16, 2022 19:49
@github-actions
Copy link
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1276.41 ms 1292.42 ms 16.01 ms
Size 8.16 MiB 9.16 MiB 1.00 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
04db237 1273.29 ms 1306.50 ms 33.21 ms
559d28f 1265.04 ms 1288.96 ms 23.92 ms
4efee31 1270.33 ms 1285.75 ms 15.42 ms
eb1a7c1 1281.25 ms 1295.40 ms 14.15 ms
0db91cc 1267.63 ms 1279.69 ms 12.06 ms
6957bfd 1283.80 ms 1289.00 ms 5.20 ms
870f5eb 1267.78 ms 1286.86 ms 19.08 ms
a49594a 1284.83 ms 1313.29 ms 28.45 ms
6d317ea 1277.27 ms 1287.47 ms 10.20 ms
f922f8f 1249.53 ms 1266.51 ms 16.98 ms

App size

Revision Plain With Sentry Diff
04db237 8.15 MiB 9.13 MiB 1003.16 KiB
559d28f 8.15 MiB 9.12 MiB 987.32 KiB
4efee31 8.15 MiB 9.12 MiB 991.35 KiB
eb1a7c1 8.15 MiB 9.13 MiB 1000.10 KiB
0db91cc 8.15 MiB 9.15 MiB 1018.56 KiB
6957bfd 8.15 MiB 9.15 MiB 1020.71 KiB
870f5eb 8.15 MiB 9.13 MiB 1000.08 KiB
a49594a 8.16 MiB 9.16 MiB 1.00 MiB
6d317ea 8.15 MiB 9.12 MiB 986.26 KiB
f922f8f 8.15 MiB 9.13 MiB 1003.20 KiB

@marandaneto marandaneto merged commit 453e1bc into main Nov 16, 2022
@marandaneto marandaneto deleted the fix/vendor-id branch November 16, 2022 20:24
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.

4 participants