Skip to content

[animations] Remove .flutter-plugins reference from example app #8002

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

Merged
merged 12 commits into from
Nov 8, 2024

Conversation

camsim99
Copy link
Contributor

@camsim99 camsim99 commented Nov 1, 2024

Removes .flutter-plugins reference from example by (1) deleting the packages/animations/example/android directory, (2) running (in the packages/animations directory)

flutter create example --platforms android -a kotlin --org "dev.flutter.packages.animations"

and then (3) manually made the following changes:

  • Added back packages/animations/example/android/.pluginToolsConfig.yaml (removed by command)
  • Updated the Gradle version from 8.7 to 8.3 (downgraded by command)
  • Deleted example/analysis_options.yaml (added by command)
  • Deleted example/test/ (added by command)
  • Added back artifact hub
  • Removed template TODOs
  • Bumped Kotlin Gradle version to 1.9.0
  • Bumped AGP version from 8.1.0 to 8.5.1.

Part of flutter/flutter#157660.

Pre-launch Checklist

@camsim99
Copy link
Contributor Author

camsim99 commented Nov 5, 2024

I believe this should be changelog/version exempt.

@camsim99 camsim99 marked this pull request as ready for review November 5, 2024 18:45
@camsim99 camsim99 requested a review from goderbauer as a code owner November 5, 2024 18:45
Copy link
Contributor

@matanlurey matanlurey left a comment

Choose a reason for hiding this comment

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

Looks good and hopefully mechanically easy! Thank you!

}
}
namespace 'dev.flutter.packages.animations.example'
lint {
disable 'InvalidPackage'
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, we don't need this any more? It was added relatively recently (#3574)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I didn't catch this when I was adding back removed lines. I'll add it back.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see any failed Android lint tests though 🤷🏻 Maybe @reidbaker remembers why we needed this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I dont remember reading the definition also didnt jog my memory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's try removing it then :)

if (System.getenv().containsKey(artifactRepoKey)) {
println "Using artifact hub"
maven { url System.getenv(artifactRepoKey) }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll need to add the new artifact hub code; see https://github.com/flutter/packages/pull/7822/files for an example.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@stuartmorgan-g
Copy link
Contributor

test-exempt: code refactor with no semantic change

@jesswrd jesswrd self-requested a review November 7, 2024 17:56
@camsim99
Copy link
Contributor Author

camsim99 commented Nov 7, 2024

Ended up having to modify the gradle-check command to accept namespace = 'namespace.name.here'/namespace = "namespace.name.here" as well as namespace 'namespace.name.here'.

@camsim99 camsim99 added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Nov 7, 2024
Copy link
Contributor

@reidbaker reidbaker left a comment

Choose a reason for hiding this comment

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

I think one regex is better than 2 for this situation.

@camsim99 camsim99 added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Nov 8, 2024
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM

}
plugins {
id "dev.flutter.flutter-plugin-loader" version "1.0.0"
id "com.android.application" version "8.1.0" apply false
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the AGP version, right? The app was 8.5.1 before, so we should probably keep that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!!

@camsim99 camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 8, 2024
@auto-submit auto-submit bot merged commit a3ea39d into flutter:main Nov 8, 2024
76 checks passed
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 8, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 13, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 13, 2024
flutter/packages@72356fd...26e123a

2024-11-13 [email protected] [camera_windows] Set device media type for video preview explicitly (flutter/packages#7447)
2024-11-13 [email protected] [go_router] Add support for relative routes (flutter/packages#6825)
2024-11-13 [email protected] [vector_graphics_compiler] fix a renamed method parameter lint (flutter/packages#8070)
2024-11-12 [email protected] [in_app_purchase] Add expiration date to Transaction (flutter/packages#8030)
2024-11-12 [email protected] [various] Clean up contributing guides (flutter/packages#8032)
2024-11-12 [email protected] [ci] Remove web renderer option from tools. (flutter/packages#8055)
2024-11-12 [email protected] [url_launcher] Update Pigeon version for Linux (flutter/packages#8065)
2024-11-12 [email protected] [go_router] Add support for preloading branches of StatefulShellRoute (revised solution) (flutter/packages#6467)
2024-11-12 [email protected] [pigeon] Make Linux type declarations public (flutter/packages#8040)
2024-11-11 [email protected] Roll Flutter from 73546b3 to c8510f2 (30 revisions) (flutter/packages#8042)
2024-11-11 [email protected] Use dependabot multi-directory configuration for Android package updates (flutter/packages#8048)
2024-11-11 [email protected] [tools] Run `pub get` before `format` (flutter/packages#8052)
2024-11-11 [email protected] [file_selector] Fix Linux cancel regression (flutter/packages#8051)
2024-11-09 [email protected] [shared_preferences] Fix confusing language in README (flutter/packages#8049)
2024-11-08 [email protected] Use dependabot multi-directory configuration for Android example gradle updates (flutter/packages#8036)
2024-11-08 [email protected] [animations] Remove `.flutter-plugins` reference from example app (flutter/packages#8002)
2024-11-08 [email protected] Group dependabot github-action update PRs, delete dead docker updates (flutter/packages#8044)
2024-11-08 [email protected] [vector_graphics_compiler] fix-null-exception (flutter/packages#8006)
2024-11-08 [email protected] [tools] Format Dart per-package (flutter/packages#8043)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
override: no changelog needed Override the check requiring CHANGELOG updates for most changes override: no versioning needed Override the check requiring version bumps for most changes p: animations platform-android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants