-
-
Notifications
You must be signed in to change notification settings - Fork 278
ci: fix native binding generation #3222
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
Conversation
Android Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 0929dbf | 462.82 ms | 492.76 ms | 29.94 ms |
| ec78888 | 457.94 ms | 519.96 ms | 62.02 ms |
| de377fd | 572.35 ms | 589.21 ms | 16.87 ms |
| 5b9a0da | 446.94 ms | 449.22 ms | 2.28 ms |
| 765aa8b | 493.51 ms | 531.23 ms | 37.72 ms |
| 3615e19 | 468.38 ms | 504.71 ms | 36.33 ms |
| 8825ed8 | 447.65 ms | 456.90 ms | 9.25 ms |
| 0fb45d0 | 482.79 ms | 554.02 ms | 71.23 ms |
| dbd526b | 504.88 ms | 569.02 ms | 64.15 ms |
| f761369 | 462.73 ms | 563.80 ms | 101.06 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 0929dbf | 6.54 MiB | 7.70 MiB | 1.17 MiB |
| ec78888 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
| de377fd | 6.54 MiB | 7.71 MiB | 1.17 MiB |
| 5b9a0da | 13.93 MiB | 14.93 MiB | 1.00 MiB |
| 765aa8b | 6.54 MiB | 7.70 MiB | 1.16 MiB |
| 3615e19 | 6.54 MiB | 7.70 MiB | 1.16 MiB |
| 8825ed8 | 13.93 MiB | 14.93 MiB | 1.00 MiB |
| 0fb45d0 | 6.54 MiB | 7.70 MiB | 1.17 MiB |
| dbd526b | 6.54 MiB | 7.69 MiB | 1.15 MiB |
| f761369 | 6.54 MiB | 7.70 MiB | 1.16 MiB |
iOS Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 81f83eb | 1259.53 ms | 1273.39 ms | 13.86 ms |
| 93b7728 | 1247.23 ms | 1264.87 ms | 17.64 ms |
| 827bf09 | 1261.86 ms | 1276.41 ms | 14.55 ms |
| eca355d | 1238.39 ms | 1266.98 ms | 28.59 ms |
| 79f6b41 | 1269.33 ms | 1279.71 ms | 10.38 ms |
| dbd526b | 1244.78 ms | 1259.02 ms | 14.24 ms |
| 73dca78 | 1246.65 ms | 1265.42 ms | 18.76 ms |
| c26ed0a | 1244.11 ms | 1263.85 ms | 19.75 ms |
| cc4e375 | 1253.06 ms | 1263.81 ms | 10.75 ms |
| b6c8720 | 1252.65 ms | 1266.61 ms | 13.96 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 81f83eb | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| 93b7728 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| 827bf09 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| eca355d | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| 79f6b41 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| dbd526b | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| 73dca78 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| c26ed0a | 5.53 MiB | 5.97 MiB | 453.76 KiB |
| cc4e375 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| b6c8720 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3222 +/- ##
==========================================
+ Coverage 87.86% 89.84% +1.98%
==========================================
Files 289 94 -195
Lines 9899 3389 -6510
==========================================
- Hits 8698 3045 -5653
+ Misses 1201 344 -857 ☔ View full report in Codecov by Sentry. |
denrase
left a comment
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, just one question/suggestion.
| elseif ($IsMacOS) | ||
| { | ||
| $flutterZip = Join-Path $tempRoot "flutter.zip" | ||
| Invoke-WebRequest -Uri "https://storage.googleapis.com/flutter_infra_release/releases/stable/macos/flutter_macos_3.27.3-stable.zip" -OutFile $flutterZip |
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.
Should we inject/read the Flutter version instead of setting fixed 3.27.3-stable?
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 think we went over this when I added support for updating the Sentry JS sdk, we'd have to modify the https://github.com/getsentry/github-workflows/tree/main/updater to allow custom steps to run (e.g use setup flutter action) before the update job runs.
all in all doable but not really a priority right now
#skip-changelog
previously we never set up Flutter so the binding generation just failed silently
see the created PR which now has proper binding generation: #3223