Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[ci] Run analysis with older versions of Flutter #5000

Merged
merged 15 commits into from
Mar 9, 2022

Conversation

stuartmorgan
Copy link
Contributor

@stuartmorgan stuartmorgan commented Mar 4, 2022

As a sanity check to reduce accidentally publishing plugins that claim to work on versions we no longer officially support, but with Flutter pubspec constraints that will cause them to be picked up by old versions, this runs analyze with the previous two stable releases. While this won't catch all possible issues (runtime failures, use of new Flutter APIs in native code), it will catch a whole category of such errors with relatively little additional CI cost.

While this does require manual updates to pubspec.yaml periodically, since this is best-effort testing such updates aren't time-critical, as the impact of forgetting is minimal.

Restructures the tool's analyze command to run flutter packages get during package iteration, instead of pre-iterating everything, to allow for skipping packages by version without having to duplicate logic. (This also has the advantage that failures in that step will be assigned to the specific package that causes the problem, as with most failures, rather than a generic step as it was previously doing.)

No version change: Dropping support for older versions of Flutter doesn't need a release, just to be applied to future releases.

Part of flutter/flutter#98697

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@stuartmorgan stuartmorgan marked this pull request as ready for review March 7, 2022 21:23
@stuartmorgan
Copy link
Contributor Author

This should be ready for review (I'll just need to re-push to get publishable to update). There are a couple of cases where we have to drop support for older versions only because of the example or integration test using new functionality, which isn't ideal, but if we can't run analyze on those versions then we can't get the safety check this is meant to provide, so dropping support seems like the best option.

Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a nit

Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a nit

(forgot to add nit in last review)

script/tool/lib/src/common/package_looping_command.dart Outdated Show resolved Hide resolved
.cirrus.yml Outdated
Comment on lines 151 to 155
matrix:
CHANNEL: "2.5.3"
CHANNEL: "2.8.1"
analyze_script:
- ./script/tool_runner.sh analyze --skip-not-supporting-flutter-version="$CHANNEL" --custom-analysis=script/configs/custom_analysis.yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great stuff! Thanks for adding this configurability to the package_looping_command!

@stuartmorgan stuartmorgan added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Mar 9, 2022
@fluttergithubbot fluttergithubbot merged commit 6852d0a into flutter:main Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 10, 2022
@stuartmorgan stuartmorgan deleted the analyze-old-flutter branch April 19, 2022 17:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p: camera p: google_sign_in p: quick_actions p: video_player platform-web waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants