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.

Sign up for GitHub

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

Jump to bottom

Remove the use-flutter configuration #284

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Remove the use-flutter configuration #284

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
951dec1
Remove the use-flutter configuration
natebosch Aug 7, 2024
f15b1a6
Remove another use of the argument
natebosch Aug 7, 2024
dcf6c38
Bump version and add changelog
natebosch Aug 7, 2024
f3d637e
Remove use-flutter from the health workflow
natebosch Aug 7, 2024
be67a40
Retain the workflow arguments
natebosch Aug 7, 2024
9d72b6b
Restore workflow argument in publish, omit from internal workflow
natebosch Aug 7, 2024
71c9d83
Remove another use of --use-flutter
natebosch Aug 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions .github/workflows/health.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -124,7 +121,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 }}

Expand All @@ -137,7 +133,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 }}

Expand All @@ -150,7 +145,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 }}
Expand All @@ -166,7 +160,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 }}
Expand All @@ -181,7 +174,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 }}

Expand All @@ -194,7 +186,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 }}

Expand All @@ -207,7 +198,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 }}

Expand Down
6 changes: 0 additions & 6 deletions .github/workflows/health_base.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,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
Expand Down
30 changes: 0 additions & 30 deletions .github/workflows/publish.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

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

@devoncarew @mosuem Have we made breaking changes to firehose CLI before? How do we handle cases where a repo could be using an older publish.yaml file than what dart pub global activate firehose installs?

Copy link
Member Author

Choose a reason for hiding this comment

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

We recommend using @main, so as long as we publish shortly after landing this on the main branch, I think it should work out for most use cases.

Is it OK if we break configuration that are pinning the publish workflow?

Copy link
Member

Choose a reason for hiding this comment

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

I don't believe we've made breaking CLI changes before.

Repos generally always reference the .github/workflows/publish.yaml script at head (i.e., uses: dart-lang/ecosystem/.github/workflows/publish.yaml@main). That then globally activates firehose - that latest published version. We update the various publish.yaml clients to remove any use of use-flutter, but I don't think we'll have any other work to do.

Copy link
Member

Choose a reason for hiding this comment

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

I think breaks are ok; we should have some good text for the changelog, and likely rev. to a new 'major' version to help call out the change.

Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion .github/workflows/publish_internal.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,4 @@ jobs:
uses: ./.github/workflows/publish.yaml
with:
local_debug: true
use-flutter: false
write-comments: false
5 changes: 5 additions & 0 deletions pkgs/firehose/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
1 change: 0 additions & 1 deletion pkgs/firehose/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"` |
Expand Down
9 changes: 1 addition & 8 deletions pkgs/firehose/bin/firehose.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> arguments) async {
var argParser = _createArgs();
Expand All @@ -25,7 +24,6 @@ void main(List<String> 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<String>)
.where((pattern) => pattern.isNotEmpty)
.map((pattern) => Glob(pattern, recursive: true))
Expand All @@ -45,7 +43,7 @@ void main(List<String> arguments) async {
exit(1);
}

final firehose = Firehose(Directory.current, useFlutter, ignoredPackages);
final firehose = Firehose(Directory.current, ignoredPackages);

if (validate) {
await firehose.validate();
Expand Down Expand Up @@ -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.',
Expand Down
11 changes: 2 additions & 9 deletions pkgs/firehose/lib/firehose.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ const String _ignoreWarningsLabel = 'publish-ignore-warnings';

class Firehose {
final Directory directory;
final bool useFlutter;
final List<Glob> ignoredPackages;

Firehose(this.directory, this.useFlutter, this.ignoredPackages);
Firehose(this.directory, this.ignoredPackages);

/// Validate the packages in the repository.
///
Expand Down Expand Up @@ -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',
Expand Down
4 changes: 1 addition & 3 deletions pkgs/firehose/lib/src/health/health.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,7 @@ class Health {
};

Future<HealthCheckResult> validateCheck() async {
//TODO: Add Flutter support for PR health checks
Copy link
Member Author

Choose a reason for hiding this comment

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

Most of the uses of use-flutter: were for health checks - and the health check firehose action ignores that.

var results =
await Firehose(directory, false, ignoredPackages).verify(github);
var results = await Firehose(directory, ignoredPackages).verify(github);

var markdownTable = '''
| Package | Version | Status |
Expand Down
2 changes: 1 addition & 1 deletion pkgs/firehose/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
Loading