-
Notifications
You must be signed in to change notification settings - Fork 660
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
Upgrade Flutter to 3.27 and upgrade its dependencies as well #3868
base: main
Are you sure you want to change the base?
Conversation
7392abc
to
24caed4
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3868 +/- ##
==========================================
- Coverage 89.02% 89.01% -0.01%
==========================================
Files 255 255
Lines 14583 14583
==========================================
- Hits 12982 12981 -1
- Misses 1601 1602 +1 ☔ View full report in Codecov by Sentry. |
24caed4
to
07e215a
Compare
07e215a
to
0d27f18
Compare
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.
@andrei-toterman The code changes look good to me, just left a few questions. Apart from that, I also did some basic gui functional testing on linux. It seems to be fine as well.
g_autoptr(FlPluginRegistrar) screen_retriever_registrar = | ||
fl_plugin_registry_get_registrar_for_plugin(registry, "ScreenRetrieverPlugin"); | ||
screen_retriever_plugin_register_with_registrar(screen_retriever_registrar); | ||
g_autoptr(FlPluginRegistrar) screen_retriever_linux_registrar = |
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.
Does this platform specific call stems from the flutter apis change?
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.
One of the plugins that we used has split up into 3 different plugins, one per platform. That's why the changes say it changed from just screen_retriever
to screen_retriever_linux
, and from ScreenRetriever
to ScreenRetrieverMacos
and ScreenRetrieverWindows
. It's an approach in Flutter called a 'federated plugin' which allows you to override the plugin implementation for each individual platform more easily.
@@ -633,15 +665,15 @@ packages: | |||
dependency: transitive | |||
description: flutter | |||
source: sdk | |||
version: "0.0.99" | |||
version: "0.0.0" |
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.
Why does this go down to version 0?
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 tried to look into this but I couldn't find any answer. That is not a dependency we use directly, it's a dependency that Flutter itself uses. I don't know why it went to 0.0.0 unfortunately.
0d27f18
to
442ffc0
Compare
In the meantime, Flutter 3.27.2 came out, so I just updated to that instead of just to 3.27.0 |
@andrei-toterman checked the flutter commit tag (3.27.2) and did some basic gui functional testing. Things are fine. |
MULTI-1725