Repository navigation
Plugins CI should run for each framework change #33440
Description
Breaking changes in the framework sometimes break plugins code.
As the flutter/plugins plugins are running against master on CI, but are required to also work on latest stable it often makes it challenging to "fix" the issue in the plugins repository. 2 recent examples:
- Add template type parameter to
invokeMethodcalls in the plugins repo #26431 - Revive the "set marker icon" sample for Google Maps #33438
I suggest that we block landing framework PRs that are breaking the plugins repository. While seemingly this might leave the author with a chicken-and-egg problem(how can they update the plugin code to adapt to the breaking change before the framework change has landed), but not blocking landing the change just moves the issue downstream(we can't update the plugin code before the breaking change made it to stable).
So we should solve this chicken-and-egg issue early on when there's still the option to re-plan the landing process(e.g land the change in 2 phases, the second phase lands only after the first phase made it to stable).
Activity
amirh commentedon May 28, 2019
Adding to that - while
flutter/pluginscan be used as a canary for the entire plugin ecosystem, it doesn't cover the entire ecosystem. e.g if some plugin out there is using the ImageStreamListener in their code(not in the example app) the author must choose whether to support Flutter stable or master.@Hixie what do you think about amending the framework breaking change policy, requiring that it must be possible for code that's using the changed API to be adapted such that it works both after the change, but also with the stable channel prior to the change. This will force us to land breaking changes in a phased way (e.g add the new
ImageStreamListenerAPI, and only remove the one we're deprecating after the change has made it to stable). Or find some other creative solutions(ifdefs?) to avoid the chicken-and-egg problem.Hixie commentedon May 30, 2019
I think as a general rule that may be overly strict, but certainly we should have a policy of not making breaking changes in such a way that we are forced to break our plugin compatibility policy!
Hixie commentedon May 30, 2019
See also dart-lang/sdk#35718
amirh commentedon May 30, 2019
So maybe adding something like this to the breaking change guidelines?
When planning a breaking change, consider potential plugin code that is affected by the change. Remember that plugins must be working with both Flutter stable and master channels, some breaking changes(e.g renaming a method) can force plugins to choose between supporting stable or master, in these cases land your change in a way that allows plugin authors to support both channels at any given point of time(e.g first add a method with a new name, and after that change is in stable remove the previously named method)
Hixie commentedon Jun 3, 2019
I think so long as we have tests that run the plugins against master and against tip of tree, and we make those tests run on precommit for framework tests, we should automatically cover this situation without needing more prose in the (already quite long) style guide.
amirh commentedon Jun 3, 2019
I'm not sure that running tests against
flutter/pluginsis enough(we should do it nevertheless). When making a breaking change we need to think of a plugin out there that is using the framework feature we are breaking even if non of theflutter/pluginsplugins are using it.Hixie commentedon Jun 3, 2019
If they care about not being broken, they should also add there tests to our precommit tests. That's what the test framework I'm working on is all about.
amirh commentedon Jun 3, 2019
What about non open source plugins? or plugins that cannot be added to
flutter/testsfor some other reason?stuartmorgan-g commentedon Jun 24, 2022
Running the just the analysis tests (as we are already doing) seems like the right tradeoff of catching things early without a huge extra test burden on flutter/flutter (longer test times, more flake, etc.). Breakage outside of analyze is relatively rare.
Given that, I'm going to close this as complete.
github-actions commentedon Jul 8, 2022
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.