Skip to content
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

Use dependabot multi-directory configuration for Android example gradle updates #8036

Merged
merged 3 commits into from
Nov 8, 2024

Conversation

jmagman
Copy link
Member

@jmagman jmagman commented Nov 8, 2024

Multi-directory config allows duplicated dependabot configs can be removed in favor of one that specifies multiple directories. Update the Android example apps so they can be updated all at once.

Also updated the dependabot CI command to look for either directory or directories key.

Part of flutter/flutter#148098

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@jmagman jmagman self-assigned this Nov 8, 2024
@jmagman jmagman marked this pull request as ready for review November 8, 2024 06:43
@jmagman jmagman changed the title Use dependabot multi-directory configuration for Android example gradle updates Use dependabot multi-directory configuration for Android gradle updates Nov 8, 2024
commit-message:
prefix: "[animations]"
prefix: "[gradle]"
Copy link
Member Author

Choose a reason for hiding this comment

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

@stuartmorgan @reidbaker This change would mean we lose the PR title prefix, which is unfortunate. This seems worth it, but maybe some tooling/bot is relying on this prefix matching a package name?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe some tooling/bot is relying on this prefix matching a package name?

Shouldn't be, the prefixing convention in the repo is just for our convenience when wading through PRs and issues.

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM if Reid is happy with it. I'm a bit skeptical that grouping all the non-example ones together will be easier in practice, since usually some plugins are fine and some need changes, but I know the current system has some issues too.

@jmagman
Copy link
Member Author

jmagman commented Nov 8, 2024

LGTM if Reid is happy with it. I'm a bit skeptical that grouping all the non-example ones together will be easier in practice, since usually some plugins are fine and some need changes, but I know the current system has some issues too.

This PR doesn't implement groups, which would combine the different dependencies into the same PR (what #8044 is doing).

This PR is only to deduplicate the configs (nicer anyway) so I can start adding groups in one place common across all packages. I'll work with Reid to figure out which groups make sense.

@stuartmorgan
Copy link
Contributor

Thanks, I totally missed that distinction :)

(I suspect a group for all example apps would actually be pretty nice.)

@jmagman
Copy link
Member Author

jmagman commented Nov 8, 2024

This PR is only to deduplicate the configs (nicer anyway)

I think this change will also mean for a given dependency bump there will be one PR that updates all the packages, instead of one PR per package.
For example all of these would be grouped into one Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 PR:
#7902
#7903
#7905
#7906

I'm not 100% sure though, we can see what it does...

@stuartmorgan
Copy link
Contributor

I think this change will also mean for a given dependency bump there will be one PR that updates all the packages

That's what I thought originally. If so, almost certainly good for examples, TBD for non-examples.

@jmagman
Copy link
Member Author

jmagman commented Nov 8, 2024

Wait, is dependabot working at all for examples? I know it's only supposed to work for major versions, but I see zero dependabot PRs bumping it.

Example commit history https://github.com/flutter/packages/commits/main/packages/google_maps_flutter/google_maps_flutter/example/android/app/build.gradle

Why would @gmackall have had to do part of this manually? Edit: oh because it's only a minor bump.
https://github.com/flutter/packages/pull/7521/files#diff-c40bf8281656da0025c18216f38b473c0f0c018ccce9b04c57cf25f8a391b91fL8

@jmagman jmagman marked this pull request as draft November 8, 2024 20:19
@jmagman jmagman changed the title Use dependabot multi-directory configuration for Android gradle updates Use dependabot multi-directory configuration for Android example gradle updates Nov 8, 2024
@jmagman
Copy link
Member Author

jmagman commented Nov 8, 2024

I'm going to swap this to just update the examples, though it seems like dependabot hasn't actually been needed to update the major versions (or possibly isn't working?).

Reid suggests we should do all of them and see what happens, so I'll follow up the rest in another PR that we can easily revert.

@jmagman jmagman marked this pull request as ready for review November 8, 2024 21:32
@jmagman jmagman added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 8, 2024
@auto-submit auto-submit bot merged commit 2eedd7d 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
@jmagman jmagman deleted the dependabot-gradle branch November 8, 2024 23:20
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 11, 2024
auto-submit bot pushed a commit that referenced this pull request Nov 11, 2024
…tes (#8048)

Follow up to #8036 (comment).

[Multi-directory config](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) allows duplicated dependabot configs can be removed in favor of one that specifies multiple directories.  Update Android packages so the same gradle dependencies can be updated all at once across packages.

> The directories option in the dependabot.yml file allows you to apply Dependabot updates to multiple directories at the same time.

It's unclear whether this is a good idea.
> I'm a bit skeptical that grouping all the non-example ones together will be easier in practice, since usually some plugins are fine and some need changes, but I know the current system has some issues too.

#8036 (review)

The consensus seems to be we should try it, and revert or adjust if it makes the current system worse.

Part of flutter/flutter#148098
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 dasyad00@gmail.com [camera_windows] Set device media type for video preview explicitly (flutter/packages#7447)
2024-11-13 39979207+ThangVuNguyenViet@users.noreply.github.com [go_router] Add support for relative routes (flutter/packages#6825)
2024-11-13 pq@users.noreply.github.com [vector_graphics_compiler] fix a renamed method parameter lint (flutter/packages#8070)
2024-11-12 feinstein@users.noreply.github.com [in_app_purchase] Add expiration date to Transaction (flutter/packages#8030)
2024-11-12 stuartmorgan@google.com [various] Clean up contributing guides (flutter/packages#8032)
2024-11-12 ditman@gmail.com [ci] Remove web renderer option from tools. (flutter/packages#8055)
2024-11-12 stuartmorgan@google.com [url_launcher] Update Pigeon version for Linux (flutter/packages#8065)
2024-11-12 tobias@leafnode.se [go_router] Add support for preloading branches of StatefulShellRoute (revised solution) (flutter/packages#6467)
2024-11-12 stuartmorgan@google.com [pigeon] Make Linux type declarations public (flutter/packages#8040)
2024-11-11 engine-flutter-autoroll@skia.org Roll Flutter from 73546b3 to c8510f2 (30 revisions) (flutter/packages#8042)
2024-11-11 magder@google.com Use dependabot multi-directory configuration for Android package updates (flutter/packages#8048)
2024-11-11 stuartmorgan@google.com [tools] Run `pub get` before `format` (flutter/packages#8052)
2024-11-11 stuartmorgan@google.com [file_selector] Fix Linux cancel regression (flutter/packages#8051)
2024-11-09 stuartmorgan@google.com [shared_preferences] Fix confusing language in README (flutter/packages#8049)
2024-11-08 magder@google.com Use dependabot multi-directory configuration for Android example gradle updates (flutter/packages#8036)
2024-11-08 43054281+camsim99@users.noreply.github.com [animations] Remove `.flutter-plugins` reference from example app (flutter/packages#8002)
2024-11-08 magder@google.com Group dependabot github-action update PRs, delete dead docker updates (flutter/packages#8044)
2024-11-08 37028599+EArminjon@users.noreply.github.com [vector_graphics_compiler] fix-null-exception (flutter/packages#8006)
2024-11-08 stuartmorgan@google.com [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 flutter-ecosystem@google.com 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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants