From 951dec11999263d0d99699aa975f5aba0190bb30 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 7 Aug 2024 16:27:43 +0000 Subject: [PATCH 1/7] Remove the use-flutter configuration Always set up the flutter SDK and use `flutter pub publish`, even for non-flutter Dart packages. The `flutter` command can publish non-flutter packages so no behavior changes are expected during the publish action. Directly remove the argument, configuration, and all references immediately. There is no plan for a deprecation/migration phase since this only impacts CI, and the fix is trivial. --- .github/workflows/publish.yaml | 30 ------------------------------ pkgs/firehose/README.md | 1 - pkgs/firehose/bin/firehose.dart | 9 +-------- pkgs/firehose/lib/firehose.dart | 11 ++--------- 4 files changed, 3 insertions(+), 48 deletions(-) diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 2e10733e..08d588ab 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -27,16 +27,6 @@ name: Publish # with: # sdk: beta -# When using this package to publish Flutter packages, the `use-flutter` -# parameter should be set. The `sdk` parameter is then used to specify -# the Flutter SDK. -# -# jobs: -# publish: -# uses: dart-lang/ecosystem/.github/workflows/publish.yaml@main -# with: -# use-flutter: true - # When using a post_summaries.yaml workflow to post the comments, set # the write-comments parameter to false. # @@ -74,12 +64,6 @@ on: default: "stable" required: false type: string - use-flutter: - description: >- - Whether to setup Flutter in this workflow. - default: false - required: false - type: boolean write-comments: description: >- Whether to write a comment in this workflow. @@ -120,15 +104,8 @@ jobs: submodules: ${{ inputs.checkout_submodules }} - uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1 - if: ${{ inputs.use-flutter }} with: channel: ${{ inputs.sdk }} - - - - uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 - if: ${{ !inputs.use-flutter }} - with: - sdk: ${{ inputs.sdk }} - name: Install firehose run: dart pub global activate firehose @@ -146,7 +123,6 @@ jobs: run: | dart pub global run firehose \ --validate \ - ${{ fromJSON('{"true":"--use-flutter","false":"--no-use-flutter"}')[inputs.use-flutter] }} \ --ignore-packages ${{ inputs.ignore-packages }} - name: Get comment id @@ -203,15 +179,9 @@ jobs: submodules: ${{ inputs.checkout_submodules }} - uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1 - if: ${{ inputs.use-flutter }} with: channel: ${{ inputs.sdk }} - - uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 - if: ${{ !inputs.use-flutter }} - with: - sdk: ${{ inputs.sdk }} - - name: Install firehose run: dart pub global activate firehose diff --git a/pkgs/firehose/README.md b/pkgs/firehose/README.md index 9ea55a51..263960fa 100644 --- a/pkgs/firehose/README.md +++ b/pkgs/firehose/README.md @@ -198,7 +198,6 @@ jobs: | warn_on | List of strings | Which checks should not fail, but only warn | `"license,coverage,breaking,leaking"` | | upload_coverage | boolean | Whether to upload the coverage to [coveralls](https://coveralls.io/) | `true` | | coverage_web | boolean | Whether to run `dart test -p chrome` for coverage | `false` | -| use-flutter | boolean | Whether to setup Flutter in this workflow | `false` | | ignore_license | List of globs | | `"**.g.dart"` | | ignore_coverage | List of globs | Which files to ignore for the license check | `"**.mock.dart,**.g.dart"` | | ignore_packages | List of globs | Which packages to ignore | `"pkgs/helper_package"` | diff --git a/pkgs/firehose/bin/firehose.dart b/pkgs/firehose/bin/firehose.dart index 82bb7ddd..e2245775 100644 --- a/pkgs/firehose/bin/firehose.dart +++ b/pkgs/firehose/bin/firehose.dart @@ -11,7 +11,6 @@ import 'package:glob/glob.dart'; const helpFlag = 'help'; const validateFlag = 'validate'; const publishFlag = 'publish'; -const useFlutterFlag = 'use-flutter'; void main(List arguments) async { var argParser = _createArgs(); @@ -25,7 +24,6 @@ void main(List arguments) async { final validate = argResults[validateFlag] as bool; final publish = argResults[publishFlag] as bool; - final useFlutter = argResults[useFlutterFlag] as bool; final ignoredPackages = (argResults['ignore-packages'] as List) .where((pattern) => pattern.isNotEmpty) .map((pattern) => Glob(pattern, recursive: true)) @@ -45,7 +43,7 @@ void main(List arguments) async { exit(1); } - final firehose = Firehose(Directory.current, useFlutter, ignoredPackages); + final firehose = Firehose(Directory.current, ignoredPackages); if (validate) { await firehose.validate(); @@ -88,11 +86,6 @@ ArgParser _createArgs() { negatable: false, help: 'Publish any changed packages.', ) - ..addFlag( - useFlutterFlag, - negatable: true, - help: 'Whether this is a Flutter project.', - ) ..addMultiOption( 'ignore-packages', help: 'Which packages to ignore.', diff --git a/pkgs/firehose/lib/firehose.dart b/pkgs/firehose/lib/firehose.dart index 618dbb6f..3f9fdfa5 100644 --- a/pkgs/firehose/lib/firehose.dart +++ b/pkgs/firehose/lib/firehose.dart @@ -29,10 +29,9 @@ const String _ignoreWarningsLabel = 'publish-ignore-warnings'; class Firehose { final Directory directory; - final bool useFlutter; final List ignoredPackages; - Firehose(this.directory, this.useFlutter, this.ignoredPackages); + Firehose(this.directory, this.ignoredPackages); /// Validate the packages in the repository. /// @@ -273,14 +272,8 @@ Saving existing comment id $existingCommentId to file ${idFile.path}'''); required bool dryRun, required bool force, }) async { - String command; - if (useFlutter) { - command = 'flutter'; - } else { - command = 'dart'; - } return await runCommand( - command, + 'flutter', args: [ 'pub', 'publish', From f15b1a6f0bbff72419673b35edc5e1e0d8f7e7a6 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 7 Aug 2024 17:46:29 +0000 Subject: [PATCH 2/7] Remove another use of the argument --- pkgs/firehose/lib/src/health/health.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index e0cc3b07..1a18fd2b 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -129,9 +129,7 @@ class Health { }; Future validateCheck() async { - //TODO: Add Flutter support for PR health checks - var results = - await Firehose(directory, false, ignoredPackages).verify(github); + var results = await Firehose(directory, ignoredPackages).verify(github); var markdownTable = ''' | Package | Version | Status | From dcf6c3811233dc6e502fdcd018503865e6d1fc6a Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 7 Aug 2024 18:11:04 +0000 Subject: [PATCH 3/7] Bump version and add changelog --- pkgs/firehose/CHANGELOG.md | 5 +++++ pkgs/firehose/pubspec.yaml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md index d409f866..4ba1d699 100644 --- a/pkgs/firehose/CHANGELOG.md +++ b/pkgs/firehose/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.10.0 + +- Remove the `--use-flutter` CLI argument and `use-flutter` GitHub action + configuration. Always use the Flutter SDK to publish. + ## 0.9.1 - Support packages nested under a 'workspace' root package. diff --git a/pkgs/firehose/pubspec.yaml b/pkgs/firehose/pubspec.yaml index d9a079a2..9840560a 100644 --- a/pkgs/firehose/pubspec.yaml +++ b/pkgs/firehose/pubspec.yaml @@ -1,6 +1,6 @@ name: firehose description: A tool to automate publishing of Pub packages from GitHub actions. -version: 0.9.1 +version: 0.10.0 repository: https://github.com/dart-lang/ecosystem/tree/main/pkgs/firehose environment: From f3d637ed6f7a861d20356503ece2e3459adc5a1c Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 7 Aug 2024 18:26:54 +0000 Subject: [PATCH 4/7] Remove use-flutter from the health workflow The `dart` command and SDK do come with the `flutter` SDK, so it should be OK to use the flutter SDK for health checks as well. --- .github/workflows/health.yaml | 15 --------------- .github/workflows/health_base.yaml | 11 ----------- .github/workflows/publish_internal.yaml | 1 - 3 files changed, 27 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 8e95a318..dd98de3c 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -26,9 +26,6 @@ name: Health # warn_on: "license,coverage,breaking,leaking" # coverage_web: false # upload_coverage: false -# use-flutter: true -# use-flutter: true -# use-flutter: true # ignore_license: "**.g.dart" # ignore_coverage: "**.mock.dart,**.g.dart" # ignore_packages: "pkgs/helper_package" @@ -83,11 +80,6 @@ on: default: false type: boolean required: false - use-flutter: - description: Whether to setup Flutter in this workflow. - default: false - required: false - type: boolean ignore_license: description: Which files to ignore for the license check. default: "\"\"" @@ -124,7 +116,6 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -137,7 +128,6 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -150,7 +140,6 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} ignore_license: ${{ inputs.ignore_license }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -166,7 +155,6 @@ jobs: upload_coverage: ${{ inputs.upload_coverage }} coverage_web: ${{ inputs.coverage_web }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} ignore_coverage: ${{ inputs.ignore_coverage }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -181,7 +169,6 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -194,7 +181,6 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} @@ -207,7 +193,6 @@ jobs: fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} - use-flutter: ${{ inputs.use-flutter }} ignore_packages: ${{ inputs.ignore_packages }} checkout_submodules: ${{ inputs.checkout_submodules }} diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 184f70d7..c204dcf2 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -44,11 +44,6 @@ on: default: false type: boolean required: false - use-flutter: - description: Whether to setup Flutter in this workflow. - default: false - required: false - type: boolean ignore_license: description: Which files to ignore for the license check. default: "\"\"" @@ -100,14 +95,8 @@ jobs: - run: mkdir -p current_repo/output/ - uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1 - if: ${{ inputs.use-flutter }} with: channel: ${{ inputs.sdk }} - - - uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 - if: ${{ !inputs.use-flutter }} - with: - sdk: ${{ inputs.sdk }} - name: Install coverage run: dart pub global activate coverage diff --git a/.github/workflows/publish_internal.yaml b/.github/workflows/publish_internal.yaml index 514977ad..4f196da2 100644 --- a/.github/workflows/publish_internal.yaml +++ b/.github/workflows/publish_internal.yaml @@ -15,5 +15,4 @@ jobs: uses: ./.github/workflows/publish.yaml with: local_debug: true - use-flutter: false write-comments: false From be67a40917aed21dbf688060e0de06044efc6651 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 7 Aug 2024 18:33:57 +0000 Subject: [PATCH 5/7] Retain the workflow arguments This will allow backwards compatibility for the few files which are passing it, and can be removed after they are cleaned up. --- .github/workflows/health.yaml | 5 +++++ .github/workflows/health_base.yaml | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index dd98de3c..81fb1138 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -80,6 +80,11 @@ on: default: false type: boolean required: false + use-flutter: + description: Whether to setup Flutter in this workflow. + default: false + required: false + type: boolean ignore_license: description: Which files to ignore for the license check. default: "\"\"" diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index c204dcf2..932829cb 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -44,6 +44,11 @@ on: default: false type: boolean required: false + use-flutter: + description: Whether to setup Flutter in this workflow. + default: false + required: false + type: boolean ignore_license: description: Which files to ignore for the license check. default: "\"\"" From 9d72b6b8c74714b673807f8925325761b65e6c97 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 7 Aug 2024 18:52:44 +0000 Subject: [PATCH 6/7] Restore workflow argument in publish, omit from internal workflow --- .github/workflows/health_base.yaml | 5 ----- .github/workflows/publish.yaml | 6 ++++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 932829cb..c204dcf2 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -44,11 +44,6 @@ on: default: false type: boolean required: false - use-flutter: - description: Whether to setup Flutter in this workflow. - default: false - required: false - type: boolean ignore_license: description: Which files to ignore for the license check. default: "\"\"" diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 08d588ab..6910f514 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -64,6 +64,12 @@ on: default: "stable" required: false type: string + use-flutter: + description: >- + Whether to setup Flutter in this workflow. + default: false + required: false + type: boolean write-comments: description: >- Whether to write a comment in this workflow. From 71c9d83545f21ab818ddfb1e0602a221cbfa788f Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 7 Aug 2024 18:53:07 +0000 Subject: [PATCH 7/7] Remove another use of --use-flutter --- .github/workflows/publish.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 6910f514..6949b031 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -192,4 +192,4 @@ jobs: run: dart pub global activate firehose - name: Publish packages - run: dart pub global run firehose --publish ${{ fromJSON('{"true":"--use-flutter","false":"--no-use-flutter"}')[inputs.use-flutter] }} + run: dart pub global run firehose --publish