Skip to content

Roll back the AAR build experiment - #36966

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

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

Conversation

@mklim

@mklim mklim commented Jul 25, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

fluttergithubbot commented Jul 25, 2019

Copy link
Copy Markdown
Contributor

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

Michael Klimushyn
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

collinjackson commented Jul 25, 2019

Copy link
Copy Markdown
Contributor

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

blasten commented Jul 25, 2019

Copy link
Copy Markdown

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

codecov Bot commented Jul 25, 2019 •
edited
Loading

Copy link
Copy Markdown

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

@jonahwilliams jonahwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread 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,

@jonahwilliams jonahwilliams Jul 26, 2019

Copy link
Copy Markdown
Contributor

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
Michael Klimushyn @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
21 of 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