Skip to content

Roll back the AAR build experiment#36966

Merged
blasten merged 2 commits intoflutter:masterfrom
mklim:roll_back_aar_experiment
Jul 26, 2019
Merged

Roll back the AAR build experiment#36966
blasten merged 2 commits intoflutter:masterfrom
mklim:roll_back_aar_experiment

Conversation

@mklim
Copy link
Contributor

@mklim mklim commented Jul 25, 2019 •
edited
Loading

Description

This has been breaking flutter/plugins CI for the past day.

Related Issues

Tests

packages/flutter_tools/test/general.shard/features_test.dart

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 25, 2019
@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Jul 25, 2019

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

/cc @dnfield

Roll back the AAR build experiment
c7f0e7c 
This has been breaking flutter/plugins CI for the past day.
@mklim mklim requested review from amirh, blasten and collinjackson July 25, 2019 20:55
@collinjackson
Copy link
Contributor

collinjackson commented Jul 25, 2019

The smoke test here is the flutter/plugins Cirrus https://cirrus-ci.com/github/flutter/plugins/master

Getting this app to run on every Flutter framework change is #33440

It might be that the path for getting this to run on Flutter framework changes is submitting it to https://github.com/flutter/tests

@blasten
Copy link

blasten commented Jul 25, 2019

It may also need to comment out gradle_jetifier_test, gradle_plugin_dependencies_test, gradle_migrate_settings_test, build_aar_plugin_test, build_aar_module_test from dev/bots/test.dart.

@codecov
Copy link

codecov bot commented Jul 25, 2019 •
edited
Loading

Codecov Report

Merging #36966 into master will decrease coverage by 0.63%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #36966      +/-   ##
==========================================
- Coverage   55.93%   55.29%   -0.64%     
==========================================
  Files         190      190              
  Lines       17564    17564              
==========================================
- Hits         9824     9712     -112     
- Misses       7740     7852     +112
Flag Coverage Δ
#flutter_tool 55.29% <ø> (-0.64%) ⬇️
Impacted Files Coverage Δ
packages/flutter_tools/lib/src/features.dart 100% <ø> (ø) ⬆️
packages/flutter_tools/lib/src/devfs.dart 14.42% <0%> (-53.24%) ⬇️
...ages/flutter_tools/lib/src/commands/build_aot.dart 29.59% <0%> (-6.13%) ⬇️
packages/flutter_tools/lib/src/bundle.dart 78.33% <0%> (-5%) ⬇️
packages/flutter_tools/lib/src/version.dart 93.59% <0%> (-1.98%) ⬇️
packages/flutter_tools/lib/src/build_info.dart 71.27% <0%> (-1.11%) ⬇️
packages/flutter_tools/lib/src/emulator.dart 69.49% <0%> (-0.85%) ⬇️
packages/flutter_tools/lib/src/cache.dart 47.05% <0%> (-0.77%) ⬇️
.../flutter_tools/lib/src/fuchsia/fuchsia_device.dart 55.39% <0%> (-0.38%) ⬇️
.../flutter_tools/lib/src/android/android_device.dart 33.5% <0%> (-0.26%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3712e3...e948a35. Read the comment docs.

@mklim mklim requested a review from jonahwilliams July 26, 2019 00:01
jonahwilliams
jonahwilliams approved these changes Jul 26, 2019
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

packages/flutter_tools/lib/src/features.dart
name: 'Build plugins independently as AARs in app projects',
configSetting: 'enable-build-plugin-as-aar',
master: FeatureChannelSetting(
available: true,
Copy link
Contributor

@jonahwilliams jonahwilliams Jul 26, 2019

Choose a reason for hiding this comment

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

If you make this available: true. Then it can still be manually enabled with flutter config --enable-build-plugin-as-aar.

@blasten blasten merged commit 30eec11 into flutter:master Jul 26, 2019
@blasten blasten mentioned this pull request Jul 26, 2019
Closed
@mklim mklim deleted the roll_back_aar_experiment branch July 26, 2019 16:48
johnsonmh pushed a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019
@johnsonmh
Roll back the AAR build experiment (flutter#36966)
3161a73 
* Roll back the AAR build experiment

This has been breaking flutter/plugins CI for the past day.

* Remove integration tests that rely on the AAR flag
@jmagman jmagman mentioned this pull request Jul 21, 2021
Closed
22 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@collinjackson collinjackson collinjackson approved these changes

@amirh amirh Awaiting requested review from amirh

+2 more reviewers

@blasten blasten blasten approved these changes

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mklim @fluttergithubbot @collinjackson @blasten @jonahwilliams @googlebot