Add more fine grained ignore capabilities to health workflow (#329)

Add generalized ignore-per-checks.

---

- [x] I’ve reviewed the contributor guide and applied the relevant
portions to this PR.

<details>
  <summary>Contribution guidelines:</summary><br>

- See our [contributor
guide](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md)
for general expectations for PRs.
- Larger or significant changes should be discussed in an issue before
creating a PR.
- Contributions to our repos should follow the [Dart style
guide](https://dart.dev/guides/language/effective-dart) and use `dart
format`.
- Most changes should add an entry to the changelog and may need to [rev
the pubspec package
version](https://github.com/dart-lang/sdk/blob/main/docs/External-Package-Maintenance.md#making-a-change).
- Changes to packages require [corresponding
tests](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md#Testing).

Note that many Dart repos have a weekly cadence for reviewing PRs -
please allow for some latency before initial review feedback.
</details>

Author: mosuem

.github/workflows/health.yaml

@@ -28,7 +28,11 @@ name: Health
 #       upload_coverage: false
 #       flutter_packages: "pkgs/my_flutter_package"
 #       ignore_license: "**.g.dart"
+#       ignore_changelog: ""
 #       ignore_coverage: "**.mock.dart,**.g.dart"
+#       ignore_breaking: ""
+#       ignore_leaking: ""
+#       ignore_donotsubmit: ""
 #       ignore_packages: "pkgs/helper_package"
 #       checkout_submodules: false
 #       experiments: "native-assets"
@@ -97,11 +101,31 @@ on:
         default: "\"\""
         required: false
         type: string
+      ignore_changelog:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
       ignore_coverage:
         description: Which files to ignore for the coverage check.
         default: "\"\""
         required: false
         type: string
+      ignore_breaking:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
+      ignore_leaking:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
+      ignore_donotsubmit:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
       ignore_packages:
         description: Which packages to ignore.
         default: "\"\""
@@ -129,6 +153,7 @@ jobs:
       warn_on: ${{ inputs.warn_on }}
       local_debug: ${{ inputs.local_debug }}
       flutter_packages: ${{ inputs.flutter_packages }}
+      ignore_changelog: ${{ inputs.ignore_changelog }}
       ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
 
@@ -173,6 +198,7 @@ jobs:
       warn_on: ${{ inputs.warn_on }}
       local_debug: ${{ inputs.local_debug }}
       flutter_packages: ${{ inputs.flutter_packages }}
+      ignore_breaking: ${{ inputs.ignore_breaking }}
       ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
 
@@ -186,6 +212,7 @@ jobs:
       warn_on: ${{ inputs.warn_on }}
       local_debug: ${{ inputs.local_debug }}
       flutter_packages: ${{ inputs.flutter_packages }}
+      ignore_donotsubmit: ${{ inputs.ignore_donotsubmit }}
       ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
 
@@ -199,6 +226,7 @@ jobs:
       warn_on: ${{ inputs.warn_on }}
       local_debug: ${{ inputs.local_debug }}
       flutter_packages: ${{ inputs.flutter_packages }}
+      ignore_leaking: ${{ inputs.ignore_leaking }}
       ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
 

.github/workflows/health_base.yaml

@@ -62,11 +62,31 @@ on:
         default: "\"\""
         required: false
         type: string
+      ignore_changelog:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
       ignore_coverage:
         description: Which files to ignore for the coverage check.
         default: "\"\""
         required: false
         type: string
+      ignore_breaking:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
+      ignore_leaking:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
+      ignore_donotsubmit:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
       ignore_packages:
         description: Which packages to ignore.
         default: "\"\""
@@ -149,9 +169,13 @@ jobs:
             --fail_on ${{ inputs.fail_on }} \
             --warn_on ${{ inputs.warn_on }} \
             --flutter_packages ${{ inputs.flutter_packages }} \
+            --ignore_packages ${{ inputs.ignore_packages }} \
             --ignore_license ${{ inputs.ignore_license }} \
+            --ignore_changelog ${{ inputs.ignore_changelog }} \
             --ignore_coverage ${{ inputs.ignore_coverage }} \
-            --ignore_packages ${{ inputs.ignore_packages }} \
+            --ignore_breaking ${{ inputs.ignore_breaking }} \
+            --ignore_leaking ${{ inputs.ignore_leaking }} \
+            --ignore_donotsubmit ${{ inputs.ignore_donotsubmit }} \
             --experiments ${{ inputs.experiments }}
 
       - run: test -f current_repo/output/comment.md || echo $'The ${{ inputs.check }} workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md

pkgs/firehose/CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 0.10.2-wip
 
 - Don't check licenses of generated files in PR health workflow.
+- Add generalized ignore-per-checks to health workflow.
 
 ## 0.10.1
 

pkgs/firehose/README.md

@@ -229,17 +229,16 @@ jobs:
 
 | Name | Type | Description | Example |
 | ------------- | ------------- | ------------- | ------------- |
-| checks  | List of strings  | What to check for in the PR health check | `"version,changelog,license,coverage,breaking,do-not-submit,leaking"` |
-| fail_on  | List of strings  | Which checks should lead to failure | `"version,changelog,do-not-submit"` |
-| 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"` |
-| checkout_submodules  | boolean  | Whether to checkout submodules of git repositories | `false` |
-| experiments  | List of strings  | Which experiments should be enabled for Dart | `"native-assets"` |
+| `checks`  | List of strings  | What to check for in the PR health check | `"version,changelog,license,coverage,breaking,do-not-submit,leaking"` |
+| `fail_on`  | List of strings  | Which checks should lead to failure | `"version,changelog,do-not-submit"` |
+| `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` |
+| `flutter_packages`  | List of strings  | List of packages depending on Flutter | `"pkgs/intl_flutter"` |
+| `ignore_*`  | List of globs | Files to ignore, where `*` can be `license`, `changelog`, `coverage`, `breaking`, `leaking`, or `donotsubmit` | `"**.g.dart"` |
+| `ignore_packages`  | List of globs  | Which packages to ignore completely | `"pkgs/helper_package"` |
+| `checkout_submodules`  | boolean  | Whether to checkout submodules of git repositories | `false` |
+| `experiments`  | List of strings  | Which experiments should be enabled for Dart | `"native-assets"` |
 
 ### Workflow docs
 

pkgs/firehose/bin/health.dart

@@ -9,7 +9,7 @@ import 'package:firehose/src/github.dart';
 import 'package:firehose/src/health/health.dart';
 
 void main(List<String> arguments) async {
-  var checkTypes = Check.values.map((c) => c.name);
+  var checkTypes = Check.values.map((c) => c.displayName);
   var argParser = ArgParser()
     ..addOption(
       'check',
@@ -21,16 +21,6 @@ void main(List<String> arguments) async {
       defaultsTo: [],
       help: 'Which packages to ignore.',
     )
-    ..addMultiOption(
-      'ignore_license',
-      defaultsTo: [],
-      help: 'Which files to ignore for the license check.',
-    )
-    ..addMultiOption(
-      'ignore_coverage',
-      defaultsTo: [],
-      help: 'Which files to ignore for the coverage check.',
-    )
     ..addMultiOption(
       'warn_on',
       allowed: checkTypes,
@@ -54,15 +44,22 @@ void main(List<String> arguments) async {
       defaultsTo: [],
       help: 'The Flutter packages in this repo',
     );
+  for (var check in Check.values) {
+    argParser.addMultiOption(
+      'ignore_${check.name}',
+      defaultsTo: [],
+      help: 'Which files to ignore for the ${check.displayName} check.',
+    );
+  }
   final parsedArgs = argParser.parse(arguments);
   final checkStr = parsedArgs.option('check');
-  final check = Check.values.firstWhere((c) => c.name == checkStr);
+  final check = Check.values.firstWhere((c) => c.displayName == checkStr);
   final warnOn = parsedArgs.multiOption('warn_on');
   final failOn = parsedArgs.multiOption('fail_on');
   final flutterPackages = _listNonEmpty(parsedArgs, 'flutter_packages');
   final ignorePackages = _listNonEmpty(parsedArgs, 'ignore_packages');
-  final ignoreLicense = _listNonEmpty(parsedArgs, 'ignore_license');
-  final ignoreCoverage = _listNonEmpty(parsedArgs, 'ignore_coverage');
+  final ignoredFor = Map.fromEntries(Check.values
+      .map((c) => MapEntry(c, _listNonEmpty(parsedArgs, 'ignore_${c.name}'))));
   final experiments = _listNonEmpty(parsedArgs, 'experiments');
   final coverageWeb = parsedArgs.flag('coverage_web');
   if (warnOn.toSet().intersection(failOn.toSet()).isNotEmpty) {
@@ -76,8 +73,7 @@ void main(List<String> arguments) async {
     failOn,
     coverageWeb,
     ignorePackages,
-    ignoreLicense,
-    ignoreCoverage,
+    ignoredFor,
     experiments,
     GithubApi(),
     flutterPackages,

pkgs/firehose/lib/src/health/changelog.dart

@@ -12,13 +12,12 @@ import '../repo.dart';
 
 Future<Map<Package, List<GitFile>>> packagesWithoutChangelog(
   GithubApi github,
-  List<Glob> ignoredPackages,
+  List<Glob> ignored,
   Directory directory,
 ) async {
   final repo = Repository(directory);
-  final packages = repo.locatePackages(ignore: ignoredPackages);
-
-  final files = await github.listFilesForPR(directory, ignoredPackages);
+  final packages = repo.locatePackages(ignore: ignored);
+  final files = await github.listFilesForPR(directory, ignored);
 
   var packagesWithoutChangedChangelog = collectPackagesWithoutChangelogChanges(
     packages,

pkgs/firehose/lib/src/health/coverage.dart

@@ -16,38 +16,35 @@ import 'lcov.dart';
 
 class Coverage {
   final bool coverageWeb;
-  final List<Glob> ignoredFiles;
-  final List<Glob> ignoredPackages;
+  final List<Glob> ignored;
   final Directory directory;
   final List<String> experiments;
   final String dartExecutable;
 
   Coverage(
     this.coverageWeb,
-    this.ignoredFiles,
-    this.ignoredPackages,
+    this.ignored,
     this.directory,
     this.experiments,
     this.dartExecutable,
   );
 
   CoverageResult compareCoveragesFor(List<GitFile> files, Directory base) {
     var repository = Repository(directory);
-    var packages = repository.locatePackages(ignore: ignoredPackages);
+    var packages = repository.locatePackages(ignore: ignored);
     print('Found packages $packages at $directory');
 
     var filesOfInterest = files
         .where((file) => path.extension(file.filename) == '.dart')
         .where((file) => file.status != FileStatus.removed)
         .where((file) => isInSomePackage(packages, file.filename))
         .where((file) => isNotATest(packages, file.filename))
-        .where(
-            (file) => ignoredFiles.none((glob) => glob.matches(file.filename)))
+        .where((file) => ignored.none((glob) => glob.matches(file.filename)))
         .toList();
     print('The files of interest are $filesOfInterest');
 
     var baseRepository = Repository(base);
-    var basePackages = baseRepository.locatePackages(ignore: ignoredPackages);
+    var basePackages = baseRepository.locatePackages(ignore: ignored);
     print('Found packages $basePackages at $base');
 
     var changedPackages = packages