From 41f8ca7feb20d4a8a617f5d51b4c20f38b3aabad Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 11:58:58 +0100 Subject: [PATCH 01/49] Update health workflow --- pkgs/firehose/lib/src/github.dart | 10 ++++++++++ pkgs/firehose/lib/src/health/health.dart | 20 +++++++++++++++----- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/pkgs/firehose/lib/src/github.dart b/pkgs/firehose/lib/src/github.dart index f2d763ba..c37da5b7 100644 --- a/pkgs/firehose/lib/src/github.dart +++ b/pkgs/firehose/lib/src/github.dart @@ -170,4 +170,14 @@ enum FileStatus { static FileStatus fromString(String s) => FileStatus.values.firstWhere((element) => element.name == s); + + bool get isRelevant => switch (this) { + FileStatus.added => true, + FileStatus.removed => true, + FileStatus.modified => true, + FileStatus.renamed => true, + FileStatus.copied => true, + FileStatus.changed => true, + FileStatus.unchanged => false, + }; } diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 440b31fd..719c5d03 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -105,11 +105,10 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati } Future breakingCheck(GithubApi github) async { - final repo = Repository(); - final packages = repo.locatePackages(); - var changeForPackage = {}; - var baseDirectory = Directory('../base_repo'); - for (var package in packages) { + final filesInPR = await github.listFilesForPR(); + final changeForPackage = {}; + final baseDirectory = Directory('../base_repo'); + for (var package in packagesContaining(filesInPR)) { var currentPath = path.relative(package.directory.path, from: Directory.current.path); var basePackage = path.relative( @@ -347,6 +346,17 @@ Saving existing comment id $existingCommentId to file ${idFile.path}'''); exitCode = 1; } } + + List packagesContaining(List filesInPR) { + var files = filesInPR.where((element) => element.status.isRelevant); + final repo = Repository(); + return repo.locatePackages().where((package) { + var relativePackageDirectory = + path.relative(package.directory.path, from: Directory.current.path); + return files.any( + (file) => path.isWithin(relativePackageDirectory, file.relativePath)); + }).toList(); + } } Version getNewVersion(BreakingLevel level, Version oldVersion) { From 83c278926324eba046c9c194d33c0576eeb1770b Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 13:15:23 +0100 Subject: [PATCH 02/49] Add warn_on and fail_on --- .github/workflows/health.yaml | 12 ++- pkgs/firehose/bin/health.dart | 29 ++++--- pkgs/firehose/lib/src/health/health.dart | 103 ++++++++++++++--------- 3 files changed, 95 insertions(+), 49 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 63d91eeb..005675ba 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -47,6 +47,16 @@ on: default: "version,changelog,license,coverage,breaking,do-not-submit" type: string required: false + fail_on: + description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit" + default: "version,changelog,license,coverage,breaking,do-not-submit" + type: string + required: false + warn_on: + description: Which checks should not fail, but only warn - any subset of "version,changelog,license,coverage,breaking,do-not-submit" + default: "version,changelog,license,coverage,breaking,do-not-submit" + type: string + required: false local_debug: description: Whether to use a local copy of package:firehose - only for debug default: false @@ -119,7 +129,7 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ github.event.number }} PR_LABELS: "${{ join(github.event.pull_request.labels.*.name) }}" - run: cd current_repo/ && dart pub global run firehose:health --checks ${{ inputs.checks }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} + run: cd current_repo/ && dart pub global run firehose:health --checks ${{ inputs.checks }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} --fail_on ${{ inputs.fail_on }} --warn_on ${{ inputs.warn_on }} - name: Upload coverage to Coveralls if: ${{ inputs.upload_coverage }} diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index 1f6ab49a..204a3349 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -11,23 +11,32 @@ void main(List arguments) async { var argParser = ArgParser() ..addMultiOption( 'checks', - allowed: [ - 'version', - 'license', - 'changelog', - 'coverage', - 'breaking', - 'do-not-submit', - ], + allowed: checkTypes, help: 'Check PR health.', ) + ..addMultiOption( + 'warn_on', + allowed: checkTypes, + help: 'Which checks to display warnings on', + ) + ..addMultiOption( + 'fail_on', + allowed: checkTypes, + help: 'Which checks should lead to workflow failure', + ) ..addFlag( 'coverage_web', help: 'Whether to run web tests for coverage', ); var parsedArgs = argParser.parse(arguments); var checks = parsedArgs['checks'] as List; + var warnOn = parsedArgs['warn_on'] as List; + var failOn = parsedArgs['fail_on'] as List; var coverageWeb = parsedArgs['coverage_web'] as bool; - - await Health(Directory.current).healthCheck(checks, coverageWeb); + if (warnOn.toSet().intersection(failOn.toSet()).isNotEmpty) { + throw ArgumentError('The checks for which warnings are displayed and the ' + 'checks which lead to failure must be disjoint.'); + } + await Health(Directory.current, checks, warnOn, failOn, coverageWeb) + .healthCheck(); } diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 719c5d03..c129abd7 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -38,14 +38,33 @@ const String _breakingBotTag = '### Breaking changes'; const String _prHealthTag = '## PR Health'; +const checkTypes = [ + 'version', + 'license', + 'changelog', + 'coverage', + 'breaking', + 'do-not-submit', +]; + class Health { final Directory directory; - Health(this.directory); - - Future healthCheck(List args, bool coverageweb) async { - var github = GithubApi(); - + Health( + this.directory, + this.checks, + this.warnOn, + this.failOn, + this.coverageweb, + ); + final github = GithubApi(); + + final List checks; + final List warnOn; + final List failOn; + final bool coverageweb; + + Future healthCheck() async { // Do basic validation of our expected env var. if (!expectEnv(github.repoSlug?.fullName, 'GITHUB_REPOSITORY')) return; if (!expectEnv(github.issueNumber?.toString(), 'ISSUE_NUMBER')) return; @@ -56,35 +75,39 @@ class Health { return; } - print('Start health check for the checks $args'); - var checks = [ - if (args.contains('version') && - !github.prLabels.contains('skip-validate-check')) - validateCheck, - if (args.contains('license') && - !github.prLabels.contains('skip-license-check')) - licenseCheck, - if (args.contains('changelog') && - !github.prLabels.contains('skip-changelog-check')) - changelogCheck, - if (args.contains('coverage') && - !github.prLabels.contains('skip-coverage-check')) - (GithubApi github) => coverageCheck(github, coverageweb), - if (args.contains('breaking') && - !github.prLabels.contains('skip-breaking-check')) - breakingCheck, - if (args.contains('do-not-submit') && - !github.prLabels.contains('skip-do-not-submit-check')) - doNotSubmitCheck, - ]; + print('Start health check for the checks $checks'); final results = []; - for (var check in checks) { - results.add(await check(github)); + for (final name in checkTypes) { + if (checks.contains(name) && + !github.prLabels.contains('skip-$name-check')) { + final firstResult = await checkFor(name)(); + final HealthCheckResult finalResult; + if (warnOn.contains(name) && firstResult.severity == Severity.error) { + finalResult = firstResult.withSeverity(Severity.warning); + } else if (failOn.contains(name) && + firstResult.severity == Severity.warning) { + finalResult = firstResult.withSeverity(Severity.error); + } else { + finalResult = firstResult; + } + results.add(finalResult); + } } await writeInComment(github, results); } - Future validateCheck(GithubApi github) async { + Future Function() checkFor(String checkType) => + switch (checkType) { + 'version' => validateCheck, + 'license' => licenseCheck, + 'changelog' => changelogCheck, + 'coverage' => coverageCheck, + 'breaking' => breakingCheck, + 'do-not-submit' => doNotSubmitCheck, + String() => throw ArgumentError(), + }; + + Future validateCheck() async { //TODO: Add Flutter support for PR health checks var results = await Firehose(directory, false).verify(github); @@ -104,7 +127,7 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati ); } - Future breakingCheck(GithubApi github) async { + Future breakingCheck() async { final filesInPR = await github.listFilesForPR(); final changeForPackage = {}; final baseDirectory = Directory('../base_repo'); @@ -178,7 +201,7 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} return breakingLevel; } - Future licenseCheck(GithubApi github) async { + Future licenseCheck() async { var files = await github.listFilesForPR(); var allFilePaths = await getFilesWithoutLicenses(Directory.current); @@ -222,7 +245,7 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''} ); } - Future changelogCheck(GithubApi github) async { + Future changelogCheck() async { var filePaths = await packagesWithoutChangelog(github); final markdownResult = ''' @@ -241,7 +264,7 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst ); } - Future doNotSubmitCheck(GithubApi github) async { + Future doNotSubmitCheck() async { final body = await github.pullrequestBody(); final files = await github.listFilesForPR(); print('Checking for DO_NOT${'_'}SUBMIT strings: $files'); @@ -273,11 +296,8 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} ); } - Future coverageCheck( - GithubApi github, - bool coverageWeb, - ) async { - var coverage = await Coverage(coverageWeb).compareCoverages(github); + Future coverageCheck() async { + var coverage = await Coverage(coverageweb).compareCoverages(github); var markdownResult = ''' | File | Coverage | @@ -384,6 +404,13 @@ class HealthCheckResult { final String? markdown; HealthCheckResult(this.name, this.title, this.severity, this.markdown); + + HealthCheckResult withSeverity(Severity severity) => HealthCheckResult( + name, + title, + severity, + markdown, + ); } class BreakingChange { From 8450538f6ae7b77c0f849c9bb5f9bfe86537df27 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 13:17:16 +0100 Subject: [PATCH 03/49] Change defaults --- .github/workflows/health.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 005675ba..7d817655 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -49,12 +49,12 @@ on: required: false fail_on: description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit" - default: "version,changelog,license,coverage,breaking,do-not-submit" + default: "version,changelog,do-not-submit" type: string required: false warn_on: description: Which checks should not fail, but only warn - any subset of "version,changelog,license,coverage,breaking,do-not-submit" - default: "version,changelog,license,coverage,breaking,do-not-submit" + default: "license,coverage,breaking" type: string required: false local_debug: From 9b2e772044335573358c88525996c352cba4daca Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 13:28:19 +0100 Subject: [PATCH 04/49] Refactorings --- pkgs/firehose/lib/src/health/health.dart | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index c129abd7..432421be 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -77,20 +77,24 @@ class Health { print('Start health check for the checks $checks'); final results = []; - for (final name in checkTypes) { - if (checks.contains(name) && - !github.prLabels.contains('skip-$name-check')) { - final firstResult = await checkFor(name)(); + for (final check in checks) { + print('Checking for $check'); + if (!github.prLabels.contains('skip-$check-check')) { + final firstResult = await checkFor(check)(); final HealthCheckResult finalResult; - if (warnOn.contains(name) && firstResult.severity == Severity.error) { + if (warnOn.contains(check) && firstResult.severity == Severity.error) { finalResult = firstResult.withSeverity(Severity.warning); - } else if (failOn.contains(name) && + } else if (failOn.contains(check) && firstResult.severity == Severity.warning) { finalResult = firstResult.withSeverity(Severity.error); } else { finalResult = firstResult; } results.add(finalResult); + print( + '\n\n${finalResult.severity.name.toUpperCase()}: $check done.\n\n'); + } else { + print('Skipping $check, as the skip tag is present.'); } } await writeInComment(github, results); From abc38a94abde3969e95e6e2583f6e331a2a6651f Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 13:44:56 +0100 Subject: [PATCH 05/49] Change failure message --- .github/workflows/health.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 7d817655..40f2d15c 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -150,6 +150,8 @@ jobs: name: output path: current_repo/output/ - - name: Fail the workflow if "Check PR health" failed + - name: \"Check PR health\" failed - see the logs above for the cause. if: steps.healthstep.outcome != 'success' - run: exit 1 + run: | + echo See the log of the "Check PR health" step further above to find the cause for the failure. If you want to ignore the result, you can add the `skip-${name}-tag` to the PR to not run the check. + exit 1 From d20811613bc7d3b1849fdee1614adade0599bd07 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 13:53:03 +0100 Subject: [PATCH 06/49] Write on failure --- .github/workflows/health.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 40f2d15c..4a0164c4 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -144,6 +144,9 @@ jobs: run: | mkdir -p current_repo/output/ && echo ${{ github.event.number }} > current_repo/output/issueNumber + - name: Write comment if not present + run: test -f current_repo/output/comment.md || echo "## PR Health \nThe Health workflow has encountered an exception and did not complete." >> current_repo/output/comment.md + - name: Upload folder with number and markdown uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 with: From be8d46aa5f3f23f2192f4039ed0e940ca8e64428 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 13:53:41 +0100 Subject: [PATCH 07/49] Test throw --- pkgs/firehose/lib/src/health/health.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 432421be..7372221c 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -91,6 +91,7 @@ class Health { finalResult = firstResult; } results.add(finalResult); + if (1 < 2) throw ArgumentError(); print( '\n\n${finalResult.severity.name.toUpperCase()}: $check done.\n\n'); } else { From fdae641cc36010fa3c9e0ef952aa410251cda781 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 13:56:47 +0100 Subject: [PATCH 08/49] Move comment Id saving up --- pkgs/firehose/lib/src/health/health.dart | 26 ++++++++++++++---------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 7372221c..b67192b6 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -75,6 +75,8 @@ class Health { return; } + await saveExistingCommentId(github); + print('Start health check for the checks $checks'); final results = []; for (final check in checks) { @@ -348,6 +350,18 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r var summary = '$_prHealthTag\n\n$commentText'; github.appendStepSummary(summary); + var commentFile = File('./output/comment.md'); + print('Saving comment markdown to file ${commentFile.path}'); + await commentFile.create(recursive: true); + await commentFile.writeAsString(summary); + + if (results.any((result) => result.severity == Severity.error) && + exitCode == 0) { + exitCode = 1; + } + } + + Future saveExistingCommentId(GithubApi github) async { var existingCommentId = await allowFailure( github.findCommentId(user: _githubActionsUser, searchTerm: _prHealthTag), logError: print, @@ -356,20 +370,10 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r if (existingCommentId != null) { var idFile = File('./output/commentId'); print(''' -Saving existing comment id $existingCommentId to file ${idFile.path}'''); + Saving existing comment id $existingCommentId to file ${idFile.path}'''); await idFile.create(recursive: true); await idFile.writeAsString(existingCommentId.toString()); } - - var commentFile = File('./output/comment.md'); - print('Saving comment markdown to file ${commentFile.path}'); - await commentFile.create(recursive: true); - await commentFile.writeAsString(summary); - - if (results.any((result) => result.severity == Severity.error) && - exitCode == 0) { - exitCode = 1; - } } List packagesContaining(List filesInPR) { From d1e27e3df3b9282219132503e914a0a000a51a70 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 14:02:19 +0100 Subject: [PATCH 09/49] Fix line break --- .github/workflows/health.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 4a0164c4..51371519 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -145,7 +145,7 @@ jobs: mkdir -p current_repo/output/ && echo ${{ github.event.number }} > current_repo/output/issueNumber - name: Write comment if not present - run: test -f current_repo/output/comment.md || echo "## PR Health \nThe Health workflow has encountered an exception and did not complete." >> current_repo/output/comment.md + run: test -f current_repo/output/comment.md || echo $'## PR Health \n\n The Health workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md - name: Upload folder with number and markdown uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 From babac7dc0241971a4f51791c293c99d630c8c5ea Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 14:06:15 +0100 Subject: [PATCH 10/49] Revert introducing error --- pkgs/firehose/lib/src/health/health.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index b67192b6..d2cabb89 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -93,7 +93,6 @@ class Health { finalResult = firstResult; } results.add(finalResult); - if (1 < 2) throw ArgumentError(); print( '\n\n${finalResult.severity.name.toUpperCase()}: $check done.\n\n'); } else { From c19d5433f714888d3e0c6c4a9aa0c7f009a74f8a Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 14:28:11 +0100 Subject: [PATCH 11/49] Append new workflows --- .github/workflows/coverage_internal.yaml | 15 ++++++ .github/workflows/health.yaml | 4 ++ pkgs/firehose/lib/src/github.dart | 4 +- pkgs/firehose/lib/src/health/health.dart | 61 ++++++++++++++++-------- 4 files changed, 61 insertions(+), 23 deletions(-) create mode 100644 .github/workflows/coverage_internal.yaml diff --git a/.github/workflows/coverage_internal.yaml b/.github/workflows/coverage_internal.yaml new file mode 100644 index 00000000..5cbdbedd --- /dev/null +++ b/.github/workflows/coverage_internal.yaml @@ -0,0 +1,15 @@ +# A CI configuration to check PR health. + +name: Health:Coverage +on: + pull_request: + branches: [ main ] + types: [opened, synchronize, reopened, labeled, unlabeled] +jobs: + health: + uses: ./.github/workflows/health.yaml + with: + local_debug: true + coverage_web: false + upload_coverage: false + checks: license diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 51371519..21246f67 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -96,6 +96,8 @@ jobs: with: ref: ${{ github.event.pull_request.base.ref }} path: base_repo/ + if: contains('coverage', inputs.checks) || contains('breaking', inputs.checks) + - uses: subosito/flutter-action@2783a3f08e1baf891508463f8c6653c258246225 if: ${{ inputs.use-flutter }} @@ -109,6 +111,7 @@ jobs: - name: Install coverage run: dart pub global activate coverage + if: contains('coverage', inputs.checks) - name: Install firehose run: dart pub global activate firehose @@ -120,6 +123,7 @@ jobs: - name: Install api_tool run: dart pub global activate dart_apitool + if: contains('breaking', inputs.checks) - name: Check PR health id: healthstep diff --git a/pkgs/firehose/lib/src/github.dart b/pkgs/firehose/lib/src/github.dart index c37da5b7..866bf32a 100644 --- a/pkgs/firehose/lib/src/github.dart +++ b/pkgs/firehose/lib/src/github.dart @@ -91,7 +91,7 @@ class GithubApi { /// Find a comment on the PR matching the given criteria ([user], /// [searchTerm]). Return the issue ID if a matching comment is found or null /// if there's no match. - Future findCommentId({ + Future findCommentId({ required String user, String? searchTerm, }) async { @@ -107,7 +107,7 @@ class GithubApi { }, orElse: () => null, ); - return matchingComment?.id; + return matchingComment; } Future> listFilesForPR() async => await github.pullRequests diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index d2cabb89..22b211a0 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -9,6 +9,7 @@ import 'dart:io'; import 'dart:math'; import 'package:collection/collection.dart'; +import 'package:github/src/common/model/issues.dart'; import 'package:path/path.dart' as path; import 'package:pub_semver/pub_semver.dart'; @@ -75,7 +76,7 @@ class Health { return; } - await saveExistingCommentId(github); + final issueComment = await saveExistingCommentId(github); print('Start health check for the checks $checks'); final results = []; @@ -99,9 +100,19 @@ class Health { print('Skipping $check, as the skip tag is present.'); } } - await writeInComment(github, results); + await writeInComment(github, results, issueComment); } + String tagFor(String checkType) => switch (checkType) { + 'version' => _publishBotTag2, + 'license' => _licenseBotTag, + 'changelog' => _changelogBotTag, + 'coverage' => _coverageBotTag, + 'breaking' => _breakingBotTag, + 'do-not-submit' => _doNotSubmitBotTag, + String() => throw ArgumentError(), + }; + Future Function() checkFor(String checkType) => switch (checkType) { 'version' => validateCheck, @@ -127,7 +138,6 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati return HealthCheckResult( 'validate', - _publishBotTag2, results.severity, markdownTable, ); @@ -181,7 +191,6 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati } return HealthCheckResult( 'breaking', - _breakingBotTag, changeForPackage.values.any((element) => !element.versionIsFine) ? Severity.warning : Severity.info, @@ -245,7 +254,6 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''} return HealthCheckResult( 'license', - _licenseBotTag, changedFilesPaths.isNotEmpty ? Severity.error : Severity.success, markdownResult, ); @@ -264,7 +272,6 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst return HealthCheckResult( 'changelog', - _changelogBotTag, filePaths.isNotEmpty ? Severity.error : Severity.success, markdownResult, ); @@ -296,7 +303,6 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} final hasDNS = filesWithDNS.isNotEmpty || bodyContainsDNS; return HealthCheckResult( 'do-not-submit', - _doNotSubmitBotTag, hasDNS ? Severity.error : Severity.success, hasDNS ? markdownResult : null, ); @@ -315,7 +321,6 @@ This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test- return HealthCheckResult( 'coverage', - _coverageBotTag, Severity.values[coverage.coveragePerFile.values .map((change) => change.severity.index) .fold(0, max)], @@ -326,8 +331,22 @@ This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test- Future writeInComment( GithubApi github, List results, + IssueComment? issueComment, ) async { - var commentText = + var summary = '$_prHealthTag\n\n'; + if (issueComment != null) { + var body = issueComment.body ?? ''; + for (var check in checks + .where((check) => results.none((result) => result.name == check))) { + var blockStart = body.indexOf(tagFor(check)); + if (blockStart != -1) { + var blockEnd = body.indexOf('', blockStart); + summary += body.substring(blockStart, blockEnd); + } + } + } + + final commentText = results.where((result) => result.markdown != null).map((result) { var markdown = result.markdown; var isWorseThanInfo = result.severity.index >= Severity.warning.index; @@ -343,16 +362,16 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r '''; - return '${result.title} ${result.severity.emoji}\n\n$s'; + return '${tagFor(result.name)} ${result.severity.emoji}\n\n$s'; }).join('\n'); - var summary = '$_prHealthTag\n\n$commentText'; - github.appendStepSummary(summary); + final markdownSummary = summary + commentText; + github.appendStepSummary(markdownSummary); var commentFile = File('./output/comment.md'); print('Saving comment markdown to file ${commentFile.path}'); await commentFile.create(recursive: true); - await commentFile.writeAsString(summary); + await commentFile.writeAsString(markdownSummary); if (results.any((result) => result.severity == Severity.error) && exitCode == 0) { @@ -360,19 +379,21 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r } } - Future saveExistingCommentId(GithubApi github) async { - var existingCommentId = await allowFailure( + Future saveExistingCommentId(GithubApi github) async { + var existingComment = await allowFailure( github.findCommentId(user: _githubActionsUser, searchTerm: _prHealthTag), logError: print, ); - if (existingCommentId != null) { + if (existingComment != null) { var idFile = File('./output/commentId'); print(''' - Saving existing comment id $existingCommentId to file ${idFile.path}'''); + Saving existing comment id $existingComment to file ${idFile.path}'''); await idFile.create(recursive: true); - await idFile.writeAsString(existingCommentId.toString()); + await idFile.writeAsString(existingComment.id.toString()); + return existingComment; } + return null; } List packagesContaining(List filesInPR) { @@ -407,15 +428,13 @@ enum BreakingLevel { class HealthCheckResult { final String name; - final String title; final Severity severity; final String? markdown; - HealthCheckResult(this.name, this.title, this.severity, this.markdown); + HealthCheckResult(this.name, this.severity, this.markdown); HealthCheckResult withSeverity(Severity severity) => HealthCheckResult( name, - title, severity, markdown, ); From a992afd04a7b19d40b0c110a31a97e20f3fdd06a Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 14:47:45 +0100 Subject: [PATCH 12/49] Fixes --- .github/workflows/health.yaml | 6 +++--- pkgs/firehose/test/github_test.dart | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 21246f67..0945caed 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -96,7 +96,7 @@ jobs: with: ref: ${{ github.event.pull_request.base.ref }} path: base_repo/ - if: contains('coverage', inputs.checks) || contains('breaking', inputs.checks) + if: contains(inputs.checks, 'coverage') || contains(inputs.checks, 'breaking') - uses: subosito/flutter-action@2783a3f08e1baf891508463f8c6653c258246225 @@ -111,7 +111,7 @@ jobs: - name: Install coverage run: dart pub global activate coverage - if: contains('coverage', inputs.checks) + if: contains(inputs.checks, 'coverage') - name: Install firehose run: dart pub global activate firehose @@ -123,7 +123,7 @@ jobs: - name: Install api_tool run: dart pub global activate dart_apitool - if: contains('breaking', inputs.checks) + if: contains(inputs.checks, 'breaking') - name: Check PR health id: healthstep diff --git a/pkgs/firehose/test/github_test.dart b/pkgs/firehose/test/github_test.dart index ef47b54e..ae3a195a 100644 --- a/pkgs/firehose/test/github_test.dart +++ b/pkgs/firehose/test/github_test.dart @@ -26,14 +26,14 @@ Future main() async { }); test('Find comment', () async { var commentId = await github.findCommentId(user: 'auto-submit[bot]'); - expect(commentId, 1660891263); + expect(commentId?.id, 1660891263); }); test('Find comment with searchterm', () async { var commentId = await github.findCommentId( user: 'auto-submit[bot]', searchTerm: 'before re-applying this label.', ); - expect(commentId, 1660891263); + expect(commentId?.id, 1660891263); }); test('Find comment with searchterm', () async { var commentId = await github.findCommentId( From b7c52ebf3ed7fa1159318f56d99e13267a750f32 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 14:51:27 +0100 Subject: [PATCH 13/49] Add debug --- pkgs/firehose/lib/src/health/health.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 22b211a0..8f8b0fe8 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -110,7 +110,7 @@ class Health { 'coverage' => _coverageBotTag, 'breaking' => _breakingBotTag, 'do-not-submit' => _doNotSubmitBotTag, - String() => throw ArgumentError(), + String() => throw ArgumentError('Invalid check type $checkType'), }; Future Function() checkFor(String checkType) => @@ -121,7 +121,7 @@ class Health { 'coverage' => coverageCheck, 'breaking' => breakingCheck, 'do-not-submit' => doNotSubmitCheck, - String() => throw ArgumentError(), + String() => throw ArgumentError('Invalid check type $checkType'), }; Future validateCheck() async { From 6371dc02c83412850973ecf2d335b026cc644714 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 14:53:53 +0100 Subject: [PATCH 14/49] Fix name --- pkgs/firehose/lib/src/health/health.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 8f8b0fe8..3a93c9b3 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -137,7 +137,7 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati '''; return HealthCheckResult( - 'validate', + 'version', results.severity, markdownTable, ); From 33f044679d7365bb692dc8028c295cdfdd59d57c Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 16:43:05 +0100 Subject: [PATCH 15/49] Extract license wf --- .github/workflows/health_internal.yaml | 2 +- .../workflows/{coverage_internal.yaml => license_internal.yaml} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename .github/workflows/{coverage_internal.yaml => license_internal.yaml} (93%) diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index aae8ba10..677c166f 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -12,4 +12,4 @@ jobs: local_debug: true coverage_web: false upload_coverage: false - checks: version,changelog,license,coverage,breaking,do-not-submit + checks: version,changelog,coverage,breaking,do-not-submit diff --git a/.github/workflows/coverage_internal.yaml b/.github/workflows/license_internal.yaml similarity index 93% rename from .github/workflows/coverage_internal.yaml rename to .github/workflows/license_internal.yaml index 5cbdbedd..9d96e460 100644 --- a/.github/workflows/coverage_internal.yaml +++ b/.github/workflows/license_internal.yaml @@ -1,6 +1,6 @@ # A CI configuration to check PR health. -name: Health:Coverage +name: Health:License on: pull_request: branches: [ main ] From 0a22004b38705d34322e59ba093ed8e997621896 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 16:45:57 +0100 Subject: [PATCH 16/49] Post summaries of license wf --- .github/workflows/post_summaries.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/post_summaries.yaml b/.github/workflows/post_summaries.yaml index b0dfd64f..db2ac41b 100644 --- a/.github/workflows/post_summaries.yaml +++ b/.github/workflows/post_summaries.yaml @@ -8,6 +8,7 @@ on: workflows: - Publish - Health + - Health:License types: - completed From 1b5359c23caee606f30bb51fff11bb26141207a3 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 16:58:49 +0100 Subject: [PATCH 17/49] Refactor --- pkgs/firehose/lib/src/health/health.dart | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 3a93c9b3..d9a1fdb5 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -9,7 +9,7 @@ import 'dart:io'; import 'dart:math'; import 'package:collection/collection.dart'; -import 'package:github/src/common/model/issues.dart'; +import 'package:github/github.dart' as gh; import 'package:path/path.dart' as path; import 'package:pub_semver/pub_semver.dart'; @@ -76,8 +76,6 @@ class Health { return; } - final issueComment = await saveExistingCommentId(github); - print('Start health check for the checks $checks'); final results = []; for (final check in checks) { @@ -100,7 +98,7 @@ class Health { print('Skipping $check, as the skip tag is present.'); } } - await writeInComment(github, results, issueComment); + await writeInComment(github, results); } String tagFor(String checkType) => switch (checkType) { @@ -329,10 +327,8 @@ This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test- } Future writeInComment( - GithubApi github, - List results, - IssueComment? issueComment, - ) async { + GithubApi github, List results) async { + final issueComment = await saveExistingCommentId(github); var summary = '$_prHealthTag\n\n'; if (issueComment != null) { var body = issueComment.body ?? ''; @@ -379,7 +375,7 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r } } - Future saveExistingCommentId(GithubApi github) async { + Future saveExistingCommentId(GithubApi github) async { var existingComment = await allowFailure( github.findCommentId(user: _githubActionsUser, searchTerm: _prHealthTag), logError: print, From c0669d0156a198ab463da3964041af7cfb9bf936 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 4 Jan 2024 17:21:41 +0100 Subject: [PATCH 18/49] Split into jobs --- .github/workflows/health_internal.yaml | 39 +++++++++++++++++++++++-- .github/workflows/license_internal.yaml | 15 ---------- 2 files changed, 37 insertions(+), 17 deletions(-) delete mode 100644 .github/workflows/license_internal.yaml diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index 677c166f..1061357e 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -6,10 +6,45 @@ on: branches: [ main ] types: [opened, synchronize, reopened, labeled, unlabeled] jobs: - health: + version: uses: ./.github/workflows/health.yaml with: local_debug: true coverage_web: false upload_coverage: false - checks: version,changelog,coverage,breaking,do-not-submit + checks: version + license: + uses: ./.github/workflows/health.yaml + with: + local_debug: true + coverage_web: false + upload_coverage: false + checks: license + changelog: + uses: ./.github/workflows/health.yaml + with: + local_debug: true + coverage_web: false + upload_coverage: false + checks: changelog + coverage: + uses: ./.github/workflows/health.yaml + with: + local_debug: true + coverage_web: false + upload_coverage: false + checks: coverage + breaking: + uses: ./.github/workflows/health.yaml + with: + local_debug: true + coverage_web: false + upload_coverage: false + checks: breaking + do-not-submit: + uses: ./.github/workflows/health.yaml + with: + local_debug: true + coverage_web: false + upload_coverage: false + checks: do-not-submit diff --git a/.github/workflows/license_internal.yaml b/.github/workflows/license_internal.yaml deleted file mode 100644 index 9d96e460..00000000 --- a/.github/workflows/license_internal.yaml +++ /dev/null @@ -1,15 +0,0 @@ -# A CI configuration to check PR health. - -name: Health:License -on: - pull_request: - branches: [ main ] - types: [opened, synchronize, reopened, labeled, unlabeled] -jobs: - health: - uses: ./.github/workflows/health.yaml - with: - local_debug: true - coverage_web: false - upload_coverage: false - checks: license From 4b35e55b3a11aa1f2401b9b3f43624975f4b9934 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 10:34:10 +0100 Subject: [PATCH 19/49] Separate jobs --- .github/workflows/health.yaml | 97 +++++---------- .github/workflows/health_base.yaml | 151 +++++++++++++++++++++++ .github/workflows/post_summaries.yaml | 3 +- pkgs/firehose/lib/src/health/health.dart | 21 +--- 4 files changed, 189 insertions(+), 83 deletions(-) create mode 100644 .github/workflows/health_base.yaml diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 0945caed..84a3a459 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -80,85 +80,56 @@ on: type: boolean jobs: - health: - # These permissions are required for us to create comments on PRs. - permissions: - pull-requests: write + version: + if: contains(${{ inputs.checks }}, 'version') runs-on: ubuntu-latest steps: - - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + - uses: ./.github/workflows/health_base.yaml with: - path: current_repo/ - - - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - with: - ref: ${{ github.event.pull_request.base.ref }} - path: base_repo/ - if: contains(inputs.checks, 'coverage') || contains(inputs.checks, 'breaking') + sdk: ${{ inputs.sdk }} + checks: version + fail: false + local_debug: ${{ inputs.local_debug }} + license: + if: contains(${{ inputs.checks }}, 'license') - - - uses: subosito/flutter-action@2783a3f08e1baf891508463f8c6653c258246225 - if: ${{ inputs.use-flutter }} - with: - channel: ${{ inputs.sdk }} - - - uses: dart-lang/setup-dart@b64355ae6ca0b5d484f0106a033dd1388965d06d - if: ${{ !inputs.use-flutter }} + runs-on: ubuntu-latest + + steps: + - uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} + checks: license + fail: false + local_debug: ${{ inputs.local_debug }} - - name: Install coverage - run: dart pub global activate coverage - if: contains(inputs.checks, 'coverage') - - - name: Install firehose - run: dart pub global activate firehose - if: ${{ !inputs.local_debug }} + comment: + needs: [version, license] + # These permissions are required for us to create comments on PRs. + permissions: + pull-requests: write - - name: Install local firehose - run: dart pub global activate --source path current_repo/pkgs/firehose/ - if: ${{ inputs.local_debug }} - - - name: Install api_tool - run: dart pub global activate dart_apitool - if: contains(inputs.checks, 'breaking') - - - name: Check PR health - id: healthstep - if: ${{ github.event_name == 'pull_request' }} - continue-on-error: true # continue, so that the result is written as a comment. Fail in the last step - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - ISSUE_NUMBER: ${{ github.event.number }} - PR_LABELS: "${{ join(github.event.pull_request.labels.*.name) }}" - run: cd current_repo/ && dart pub global run firehose:health --checks ${{ inputs.checks }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} --fail_on ${{ inputs.fail_on }} --warn_on ${{ inputs.warn_on }} + runs-on: ubuntu-latest - - name: Upload coverage to Coveralls - if: ${{ inputs.upload_coverage }} - uses: coverallsapp/github-action@3dfc5567390f6fa9267c0ee9c251e4c8c3f18949 + steps: + - name: Download All Artifacts + uses: actions/download-artifact@f44cd7b40bfd40b6aa1cc1b9b5b7bf03d3c67110 with: - format: lcov - base-path: current_repo/ - compare-sha: ${{ github.event.pull_request.base.ref }} - allow-empty: true + path: single-comments + pattern: comment-* + merge-multiple: true - - name: Save PR number + - name: Merge all single comments run: | - mkdir -p current_repo/output/ && echo ${{ github.event.number }} > current_repo/output/issueNumber - - - name: Write comment if not present - run: test -f current_repo/output/comment.md || echo $'## PR Health \n\n The Health workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md + mkdir output + echo $'## PR Health \n\n' >> my_file.txt + cat single-comments/* >> output/comment.md + echo ${{ github.event.number }} > output/issueNumber - name: Upload folder with number and markdown uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 with: name: output - path: current_repo/output/ - - - name: \"Check PR health\" failed - see the logs above for the cause. - if: steps.healthstep.outcome != 'success' - run: | - echo See the log of the "Check PR health" step further above to find the cause for the failure. If you want to ignore the result, you can add the `skip-${name}-tag` to the PR to not run the check. - exit 1 + path: output/ diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml new file mode 100644 index 00000000..e48addce --- /dev/null +++ b/.github/workflows/health_base.yaml @@ -0,0 +1,151 @@ +# A CI configuration to check PR health. + +name: Health:Base + +# Callers of this workflow should use it as follows: +# +# name: Health +# on: +# pull_request: +# branches: [ main ] +# types: [opened, synchronize, reopened, labeled, unlabeled] +# jobs: +# health: +# uses: dart-lang/ecosystem/.github/workflows/health.yaml@main +# with: +# coverage_web: true #If the coverage should run browser tests + +# Callers may optionally specify the version of the SDK to use when running the +# health check. This can be useful if your package has a very recent minimum SDK +# constraint. This is done via the `sdk` input parameter. Note that this +# parameter is not required; it defaults to `stable` - using the most recent +# stable release of the Dart SDK. +# +# The checks can also be restricted to any subset of version, changelog, and license, +# if needed. +# +# jobs: +# health: +# uses: dart-lang/ecosystem/.github/workflows/health.yaml@main +# with: +# sdk: beta +# checks: "version,changelog,license,coverage,breaking,do-not-submit" + +on: + workflow_call: + inputs: + sdk: + description: >- + The channel, or a specific version from a channel, to install + ('2.19.0','stable', 'beta', 'dev'). Using one of the three channels + will give you the latest version published to that channel. + default: "stable" + required: false + type: string + checks: + description: What to check for in the PR health check - any subset of "version,changelog,license,coverage,breaking,do-not-submit" + default: "version,changelog,license,coverage,breaking,do-not-submit" + type: string + required: false + fail_on: + description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit" + type: boolean + required: true + warn_on: + description: Which checks should not fail, but only warn - any subset of "version,changelog,license,coverage,breaking,do-not-submit" + type: boolean + required: false + local_debug: + description: Whether to use a local copy of package:firehose - only for debug + default: false + type: boolean + required: false + upload_coverage: + description: Whether to upload the coverage to coveralls + default: true + type: boolean + required: false + coverage_web: + description: Whether to run `dart test -p chrome` for coverage + default: false + type: boolean + required: false + use-flutter: + description: >- + Whether to setup Flutter in this workflow. + default: false + required: false + type: boolean + +jobs: + health: + # These permissions are required for us to create comments on PRs. + permissions: + pull-requests: write + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + path: current_repo/ + + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + ref: ${{ github.event.pull_request.base.ref }} + path: base_repo/ + if: contains(inputs.checks, 'coverage') || contains(inputs.checks, 'breaking') + + - name: Write comment if not present + run: test -f current_repo/output/comment.md || echo $'The ${{ inputs.checks }} workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md + + - uses: subosito/flutter-action@2783a3f08e1baf891508463f8c6653c258246225 + if: ${{ inputs.use-flutter }} + with: + channel: ${{ inputs.sdk }} + + - uses: dart-lang/setup-dart@b64355ae6ca0b5d484f0106a033dd1388965d06d + if: ${{ !inputs.use-flutter }} + with: + sdk: ${{ inputs.sdk }} + + - name: Install coverage + run: dart pub global activate coverage + if: contains(inputs.checks, 'coverage') + + - name: Install firehose + run: dart pub global activate firehose + if: ${{ !inputs.local_debug }} + + - name: Install local firehose + run: dart pub global activate --source path current_repo/pkgs/firehose/ + if: ${{ inputs.local_debug }} + + - name: Install api_tool + run: dart pub global activate dart_apitool + if: contains(inputs.checks, 'breaking') + + - name: Check PR health + id: healthstep + if: ${{ github.event_name == 'pull_request' }} + continue-on-error: true # continue, so that the result is written as a comment. Fail in the last step + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + ISSUE_NUMBER: ${{ github.event.number }} + PR_LABELS: "${{ join(github.event.pull_request.labels.*.name) }}" + run: cd current_repo/ && dart pub global run firehose:health --checks ${{ inputs.checks }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} --fail_on ${{ inputs.fail_on }} --warn_on ${{ inputs.warn_on }} + + - name: Upload coverage to Coveralls + if: ${{ inputs.upload_coverage }} + uses: coverallsapp/github-action@3dfc5567390f6fa9267c0ee9c251e4c8c3f18949 + with: + format: lcov + base-path: current_repo/ + compare-sha: ${{ github.event.pull_request.base.ref }} + allow-empty: true + + - name: Upload markdown + uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 + with: + name: comment-${{ inputs.checks }} + path: current_repo/output/comment.md \ No newline at end of file diff --git a/.github/workflows/post_summaries.yaml b/.github/workflows/post_summaries.yaml index db2ac41b..10e82c91 100644 --- a/.github/workflows/post_summaries.yaml +++ b/.github/workflows/post_summaries.yaml @@ -7,8 +7,7 @@ on: workflow_run: workflows: - Publish - - Health - - Health:License + - Health:Comment types: - completed diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index d9a1fdb5..cfc02bd7 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -9,7 +9,6 @@ import 'dart:io'; import 'dart:math'; import 'package:collection/collection.dart'; -import 'package:github/github.dart' as gh; import 'package:path/path.dart' as path; import 'package:pub_semver/pub_semver.dart'; @@ -328,20 +327,8 @@ This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test- Future writeInComment( GithubApi github, List results) async { - final issueComment = await saveExistingCommentId(github); - var summary = '$_prHealthTag\n\n'; - if (issueComment != null) { - var body = issueComment.body ?? ''; - for (var check in checks - .where((check) => results.none((result) => result.name == check))) { - var blockStart = body.indexOf(tagFor(check)); - if (blockStart != -1) { - var blockEnd = body.indexOf('', blockStart); - summary += body.substring(blockStart, blockEnd); - } - } - } - + await saveExistingCommentId(github); + var summary = ''; final commentText = results.where((result) => result.markdown != null).map((result) { var markdown = result.markdown; @@ -375,7 +362,7 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r } } - Future saveExistingCommentId(GithubApi github) async { + Future saveExistingCommentId(GithubApi github) async { var existingComment = await allowFailure( github.findCommentId(user: _githubActionsUser, searchTerm: _prHealthTag), logError: print, @@ -387,9 +374,7 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r Saving existing comment id $existingComment to file ${idFile.path}'''); await idFile.create(recursive: true); await idFile.writeAsString(existingComment.id.toString()); - return existingComment; } - return null; } List packagesContaining(List filesInPR) { From 585e0c488036a79ca8a64a96b7fc493c8882952b Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 10:43:48 +0100 Subject: [PATCH 20/49] Fix --- .github/workflows/health.yaml | 4 +-- .github/workflows/health_internal.yaml | 39 ++------------------------ 2 files changed, 4 insertions(+), 39 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 84a3a459..12a452c3 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -86,7 +86,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: ./.github/workflows/health_base.yaml + - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: version @@ -98,7 +98,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: ./.github/workflows/health_base.yaml + - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: license diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index 1061357e..677c166f 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -6,45 +6,10 @@ on: branches: [ main ] types: [opened, synchronize, reopened, labeled, unlabeled] jobs: - version: + health: uses: ./.github/workflows/health.yaml with: local_debug: true coverage_web: false upload_coverage: false - checks: version - license: - uses: ./.github/workflows/health.yaml - with: - local_debug: true - coverage_web: false - upload_coverage: false - checks: license - changelog: - uses: ./.github/workflows/health.yaml - with: - local_debug: true - coverage_web: false - upload_coverage: false - checks: changelog - coverage: - uses: ./.github/workflows/health.yaml - with: - local_debug: true - coverage_web: false - upload_coverage: false - checks: coverage - breaking: - uses: ./.github/workflows/health.yaml - with: - local_debug: true - coverage_web: false - upload_coverage: false - checks: breaking - do-not-submit: - uses: ./.github/workflows/health.yaml - with: - local_debug: true - coverage_web: false - upload_coverage: false - checks: do-not-submit + checks: version,changelog,coverage,breaking,do-not-submit From 32caf70258dacc3cadd20c7c8ae4f26e3babd055 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 10:47:20 +0100 Subject: [PATCH 21/49] Fix again --- .github/workflows/health_internal.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index 677c166f..10f1b264 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -7,7 +7,7 @@ on: types: [opened, synchronize, reopened, labeled, unlabeled] jobs: health: - uses: ./.github/workflows/health.yaml + uses: dart-lang/ecosystem/.github/workflows/health.yaml with: local_debug: true coverage_web: false From 8bf8c0fcd2ee41b390e01a73178f6257c2f3ef9a Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 10:48:19 +0100 Subject: [PATCH 22/49] Set version --- .github/workflows/health.yaml | 4 ++-- .github/workflows/health_internal.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 12a452c3..980301d9 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -86,7 +86,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml + - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml@healthUpdates with: sdk: ${{ inputs.sdk }} checks: version @@ -98,7 +98,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml + - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml@healthUpdates with: sdk: ${{ inputs.sdk }} checks: license diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index 10f1b264..c5f51382 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -7,7 +7,7 @@ on: types: [opened, synchronize, reopened, labeled, unlabeled] jobs: health: - uses: dart-lang/ecosystem/.github/workflows/health.yaml + uses: dart-lang/ecosystem/.github/workflows/health.yaml@healthUpdates with: local_debug: true coverage_web: false From ea12bb7c0adbafa3a4c28c4e2cecc42a133e47d9 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 10:55:04 +0100 Subject: [PATCH 23/49] Try again --- .github/workflows/health.yaml | 31 ++++++++++---------------- .github/workflows/health_internal.yaml | 4 ++-- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 980301d9..63a0ae1a 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -81,29 +81,22 @@ on: jobs: version: + uses: dart-lang/ecosystem/.github/workflows/health_base.yaml@healthUpdates if: contains(${{ inputs.checks }}, 'version') + with: + sdk: ${{ inputs.sdk }} + checks: version + fail: false + local_debug: ${{ inputs.local_debug }} - runs-on: ubuntu-latest - - steps: - - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml@healthUpdates - with: - sdk: ${{ inputs.sdk }} - checks: version - fail: false - local_debug: ${{ inputs.local_debug }} license: if: contains(${{ inputs.checks }}, 'license') - - runs-on: ubuntu-latest - - steps: - - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml@healthUpdates - with: - sdk: ${{ inputs.sdk }} - checks: license - fail: false - local_debug: ${{ inputs.local_debug }} + uses: dart-lang/ecosystem/.github/workflows/health_base.yaml@healthUpdates + with: + sdk: ${{ inputs.sdk }} + checks: license + fail: false + local_debug: ${{ inputs.local_debug }} comment: needs: [version, license] diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index c5f51382..79fbe9e6 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -7,9 +7,9 @@ on: types: [opened, synchronize, reopened, labeled, unlabeled] jobs: health: - uses: dart-lang/ecosystem/.github/workflows/health.yaml@healthUpdates + uses: ./.github/workflows/health.yaml with: local_debug: true coverage_web: false upload_coverage: false - checks: version,changelog,coverage,breaking,do-not-submit + checks: version,changelog,license,coverage,breaking,do-not-submit \ No newline at end of file From cda8c7f8ea22d2c72718f7a848eccbd60c2f274a Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 10:57:53 +0100 Subject: [PATCH 24/49] set fail --- .github/workflows/health.yaml | 4 ++-- .github/workflows/health_base.yaml | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 63a0ae1a..da540f7d 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -86,7 +86,7 @@ jobs: with: sdk: ${{ inputs.sdk }} checks: version - fail: false + fail: contains(${{ inputs.fail_on }}, 'version') local_debug: ${{ inputs.local_debug }} license: @@ -95,7 +95,7 @@ jobs: with: sdk: ${{ inputs.sdk }} checks: license - fail: false + fail: contains(${{ inputs.fail_on }}, 'license') local_debug: ${{ inputs.local_debug }} comment: diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index e48addce..94468367 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -49,11 +49,13 @@ on: required: false fail_on: description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit" - type: boolean - required: true + default: "version,changelog,do-not-submit" + type: string + required: false warn_on: description: Which checks should not fail, but only warn - any subset of "version,changelog,license,coverage,breaking,do-not-submit" - type: boolean + default: "license,coverage,breaking" + type: string required: false local_debug: description: Whether to use a local copy of package:firehose - only for debug From f36ff91662eb7c7ac829c0f93d650183d25ac6a2 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 10:59:20 +0100 Subject: [PATCH 25/49] Fix fail --- .github/workflows/health.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index da540f7d..7b15ebe4 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -86,7 +86,7 @@ jobs: with: sdk: ${{ inputs.sdk }} checks: version - fail: contains(${{ inputs.fail_on }}, 'version') + fail_on: ${{ inputs.fail_on }} local_debug: ${{ inputs.local_debug }} license: @@ -95,7 +95,7 @@ jobs: with: sdk: ${{ inputs.sdk }} checks: license - fail: contains(${{ inputs.fail_on }}, 'license') + fail_on: ${{ inputs.fail_on }} local_debug: ${{ inputs.local_debug }} comment: From 010b90d06a3c7ae3c2c866fa6992af53dd39fe52 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 11:01:44 +0100 Subject: [PATCH 26/49] Create folder --- .github/workflows/health_base.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 94468367..168b1878 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -99,7 +99,9 @@ jobs: if: contains(inputs.checks, 'coverage') || contains(inputs.checks, 'breaking') - name: Write comment if not present - run: test -f current_repo/output/comment.md || echo $'The ${{ inputs.checks }} workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md + run: | + mkdir -p current_repo/output/ + test -f current_repo/output/comment.md || echo $'The ${{ inputs.checks }} workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md - uses: subosito/flutter-action@2783a3f08e1baf891508463f8c6653c258246225 if: ${{ inputs.use-flutter }} From f83626efbcf8c9b402d812b5fd168190faeee2e8 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 11:06:04 +0100 Subject: [PATCH 27/49] Fix commenting --- .github/workflows/health.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 7b15ebe4..e0cb61f7 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -114,10 +114,12 @@ jobs: pattern: comment-* merge-multiple: true + - run: ls -R single-comments + - name: Merge all single comments run: | mkdir output - echo $'## PR Health \n\n' >> my_file.txt + echo $'## PR Health \n\n' >> output/comment.md cat single-comments/* >> output/comment.md echo ${{ github.event.number }} > output/issueNumber From f97896650dab091e51b000b131684a37b51ea59d Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 11:09:58 +0100 Subject: [PATCH 28/49] Rename comment files --- pkgs/firehose/lib/src/health/health.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index cfc02bd7..42c942fe 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -351,7 +351,8 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r final markdownSummary = summary + commentText; github.appendStepSummary(markdownSummary); - var commentFile = File('./output/comment.md'); + var commentFile = + File('./output/comment-${results.map((e) => e.name).join('-')}.md'); print('Saving comment markdown to file ${commentFile.path}'); await commentFile.create(recursive: true); await commentFile.writeAsString(markdownSummary); From e245114c27144d5f56a0054c69ce777610b755a2 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 11:23:47 +0100 Subject: [PATCH 29/49] Route options --- .github/workflows/health.yaml | 62 +++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index e0cb61f7..4afecac6 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -81,25 +81,81 @@ on: jobs: version: - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml@healthUpdates + name: health:version if: contains(${{ inputs.checks }}, 'version') + uses: .github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: version fail_on: ${{ inputs.fail_on }} + warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} + use-flutter: ${{ inputs.use-flutter }} + + changelog: + name: health:changelog + if: contains(${{ inputs.checks }}, 'changelog') + uses: .github/workflows/health_base.yaml + with: + sdk: ${{ inputs.sdk }} + checks: changelog + fail_on: ${{ inputs.fail_on }} + warn_on: ${{ inputs.warn_on }} + local_debug: ${{ inputs.local_debug }} + use-flutter: ${{ inputs.use-flutter }} license: + name: health:license if: contains(${{ inputs.checks }}, 'license') - uses: dart-lang/ecosystem/.github/workflows/health_base.yaml@healthUpdates + uses: .github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: license fail_on: ${{ inputs.fail_on }} + warn_on: ${{ inputs.warn_on }} + local_debug: ${{ inputs.local_debug }} + use-flutter: ${{ inputs.use-flutter }} + + coverage: + name: health:coverage + if: contains(${{ inputs.checks }}, 'coverage') + uses: .github/workflows/health_base.yaml + with: + sdk: ${{ inputs.sdk }} + checks: coverage + fail_on: ${{ inputs.fail_on }} + warn_on: ${{ inputs.warn_on }} + upload_coverage: ${{ inputs.upload_coverage }} + coverage_web: ${{ inputs.coverage_web }} + local_debug: ${{ inputs.local_debug }} + use-flutter: ${{ inputs.use-flutter }} + + breaking: + name: health:breaking + if: contains(${{ inputs.checks }}, 'breaking') + uses: .github/workflows/health_base.yaml + with: + sdk: ${{ inputs.sdk }} + checks: breaking + fail_on: ${{ inputs.fail_on }} + warn_on: ${{ inputs.warn_on }} + local_debug: ${{ inputs.local_debug }} + use-flutter: ${{ inputs.use-flutter }} + + do-not-submit: + name: health:do-not-submit + if: contains(${{ inputs.checks }}, 'do-not-submit') + uses: .github/workflows/health_base.yaml + with: + sdk: ${{ inputs.sdk }} + checks: do-not-submit + fail_on: ${{ inputs.fail_on }} + warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} + use-flutter: ${{ inputs.use-flutter }} comment: - needs: [version, license] + needs: [version, changelog, license, coverage, breaking, do-not-submit] # These permissions are required for us to create comments on PRs. permissions: pull-requests: write From b90fff6fa5c3c23c12e4f6ca3b1b5f7c35c41835 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 11:40:41 +0100 Subject: [PATCH 30/49] Add comment id --- .github/workflows/health.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 4afecac6..9e884982 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -172,6 +172,18 @@ jobs: - run: ls -R single-comments + - name: Find Comment + uses: peter-evans/find-comment@ + id: fc + with: + issue-number: ${{ github.event.number }} + comment-author: github-actions[bot] + body-includes: '## PR Health' + + - name: Write comment id to file + if: ${{ steps.fc.outputs.comment-id == 0 }} + run: echo ${{ steps.fc.outputs.comment-id }} >> output/commentId + - name: Merge all single comments run: | mkdir output From 745bb7512d80dc9a192e68a729ad929a74ee3dbe Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 11:41:55 +0100 Subject: [PATCH 31/49] Set version hash --- .github/workflows/health.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 9e884982..210de624 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -173,7 +173,7 @@ jobs: - run: ls -R single-comments - name: Find Comment - uses: peter-evans/find-comment@ + uses: peter-evans/find-comment@4541d1b6b0e618acc24284e118743fc5ad0bc5de id: fc with: issue-number: ${{ github.event.number }} From 6e04cac9078f18f49bb13ea750f808f717a6fff6 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 11:50:11 +0100 Subject: [PATCH 32/49] Fix ref --- .github/workflows/health.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 210de624..a3634ded 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -83,7 +83,7 @@ jobs: version: name: health:version if: contains(${{ inputs.checks }}, 'version') - uses: .github/workflows/health_base.yaml + uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: version @@ -95,7 +95,7 @@ jobs: changelog: name: health:changelog if: contains(${{ inputs.checks }}, 'changelog') - uses: .github/workflows/health_base.yaml + uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: changelog @@ -107,7 +107,7 @@ jobs: license: name: health:license if: contains(${{ inputs.checks }}, 'license') - uses: .github/workflows/health_base.yaml + uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: license @@ -119,7 +119,7 @@ jobs: coverage: name: health:coverage if: contains(${{ inputs.checks }}, 'coverage') - uses: .github/workflows/health_base.yaml + uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: coverage @@ -133,7 +133,7 @@ jobs: breaking: name: health:breaking if: contains(${{ inputs.checks }}, 'breaking') - uses: .github/workflows/health_base.yaml + uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: breaking @@ -145,7 +145,7 @@ jobs: do-not-submit: name: health:do-not-submit if: contains(${{ inputs.checks }}, 'do-not-submit') - uses: .github/workflows/health_base.yaml + uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} checks: do-not-submit From ecbc5fc616b7b9c1e395c828da4d852c60d473f2 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 11:56:32 +0100 Subject: [PATCH 33/49] Fix commenting --- .github/workflows/health.yaml | 4 +--- .github/workflows/health_base.yaml | 2 +- pkgs/firehose/lib/src/health/health.dart | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index a3634ded..b9688f7f 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -167,8 +167,6 @@ jobs: uses: actions/download-artifact@f44cd7b40bfd40b6aa1cc1b9b5b7bf03d3c67110 with: path: single-comments - pattern: comment-* - merge-multiple: true - run: ls -R single-comments @@ -188,7 +186,7 @@ jobs: run: | mkdir output echo $'## PR Health \n\n' >> output/comment.md - cat single-comments/* >> output/comment.md + cat single-comments/* single-comments/*/* >> output/comment.md echo ${{ github.event.number }} > output/issueNumber - name: Upload folder with number and markdown diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 168b1878..ca4ae6ae 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -151,5 +151,5 @@ jobs: - name: Upload markdown uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 with: - name: comment-${{ inputs.checks }} + name: comment path: current_repo/output/comment.md \ No newline at end of file diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 42c942fe..cfc02bd7 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -351,8 +351,7 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r final markdownSummary = summary + commentText; github.appendStepSummary(markdownSummary); - var commentFile = - File('./output/comment-${results.map((e) => e.name).join('-')}.md'); + var commentFile = File('./output/comment.md'); print('Saving comment markdown to file ${commentFile.path}'); await commentFile.create(recursive: true); await commentFile.writeAsString(markdownSummary); From 85bf4a194fb472f02f470f3d4f01bf667109a2d8 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 11:58:47 +0100 Subject: [PATCH 34/49] rename comments --- .github/workflows/health_base.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index ca4ae6ae..168b1878 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -151,5 +151,5 @@ jobs: - name: Upload markdown uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 with: - name: comment + name: comment-${{ inputs.checks }} path: current_repo/output/comment.md \ No newline at end of file From 238e5b2fe632171080014ad24ca255f25750a41a Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:02:36 +0100 Subject: [PATCH 35/49] Fix merging --- .github/workflows/health.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index b9688f7f..2a295b40 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -186,7 +186,7 @@ jobs: run: | mkdir output echo $'## PR Health \n\n' >> output/comment.md - cat single-comments/* single-comments/*/* >> output/comment.md + cat single-comments/*.md single-comments/*/*.md >> output/comment.md echo ${{ github.event.number }} > output/issueNumber - name: Upload folder with number and markdown From 5a21dc9dd67a53eb153a44e87d5abb26859ec924 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:05:01 +0100 Subject: [PATCH 36/49] Remove empty --- .github/workflows/health.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 2a295b40..a7f26ec8 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -186,7 +186,7 @@ jobs: run: | mkdir output echo $'## PR Health \n\n' >> output/comment.md - cat single-comments/*.md single-comments/*/*.md >> output/comment.md + cat single-comments/*/*.md >> output/comment.md echo ${{ github.event.number }} > output/issueNumber - name: Upload folder with number and markdown From 2e29e30adee1d1434c3210c99f617986d096c0bc Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:14:58 +0100 Subject: [PATCH 37/49] Debug finding comment id --- .github/workflows/health.yaml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index a7f26ec8..b73c7f28 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -177,6 +177,27 @@ jobs: issue-number: ${{ github.event.number }} comment-author: github-actions[bot] body-includes: '## PR Health' + + + + - name: Find Comment + uses: peter-evans/find-comment@4541d1b6b0e618acc24284e118743fc5ad0bc5de + id: fc2 + with: + issue-number: ${{ github.event.number }} + comment-author: github-actions[bot] + + - name: Find Comment + uses: peter-evans/find-comment@4541d1b6b0e618acc24284e118743fc5ad0bc5de + id: fc3 + with: + issue-number: ${{ github.event.number }} + body-includes: '## PR Health' + + - run: | + echo ${{ steps.fc.outputs.comment-id }} + echo ${{ steps.fc2.outputs.comment-id }} + echo ${{ steps.fc3.outputs.comment-id }} - name: Write comment id to file if: ${{ steps.fc.outputs.comment-id == 0 }} From b55afb69923100142b89d0140174a01d9122185e Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:17:37 +0100 Subject: [PATCH 38/49] Fix --- .github/workflows/health.yaml | 23 +---------------------- .github/workflows/health_base.yaml | 1 - 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index b73c7f28..6bf71aa8 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -177,30 +177,9 @@ jobs: issue-number: ${{ github.event.number }} comment-author: github-actions[bot] body-includes: '## PR Health' - - - - - name: Find Comment - uses: peter-evans/find-comment@4541d1b6b0e618acc24284e118743fc5ad0bc5de - id: fc2 - with: - issue-number: ${{ github.event.number }} - comment-author: github-actions[bot] - - - name: Find Comment - uses: peter-evans/find-comment@4541d1b6b0e618acc24284e118743fc5ad0bc5de - id: fc3 - with: - issue-number: ${{ github.event.number }} - body-includes: '## PR Health' - - - run: | - echo ${{ steps.fc.outputs.comment-id }} - echo ${{ steps.fc2.outputs.comment-id }} - echo ${{ steps.fc3.outputs.comment-id }} - name: Write comment id to file - if: ${{ steps.fc.outputs.comment-id == 0 }} + if: ${{ steps.fc.outputs.comment-id != 0 }} run: echo ${{ steps.fc.outputs.comment-id }} >> output/commentId - name: Merge all single comments diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 168b1878..0a52a16e 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -132,7 +132,6 @@ jobs: - name: Check PR health id: healthstep if: ${{ github.event_name == 'pull_request' }} - continue-on-error: true # continue, so that the result is written as a comment. Fail in the last step env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ github.event.number }} From 797675d4fe11e7f3820fe1188664260a8078974b Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:20:34 +0100 Subject: [PATCH 39/49] Always comment --- .github/workflows/health.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 6bf71aa8..8ffd4029 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -156,6 +156,7 @@ jobs: comment: needs: [version, changelog, license, coverage, breaking, do-not-submit] + if: always() # These permissions are required for us to create comments on PRs. permissions: pull-requests: write From 0a1a9324b37019e39ef925477f3d9c61dc8f4ce1 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:23:47 +0100 Subject: [PATCH 40/49] Always upload comment --- .github/workflows/health_base.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 0a52a16e..3f60e14b 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -139,7 +139,7 @@ jobs: run: cd current_repo/ && dart pub global run firehose:health --checks ${{ inputs.checks }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} --fail_on ${{ inputs.fail_on }} --warn_on ${{ inputs.warn_on }} - name: Upload coverage to Coveralls - if: ${{ inputs.upload_coverage }} + if: ${{ always() && inputs.upload_coverage }} uses: coverallsapp/github-action@3dfc5567390f6fa9267c0ee9c251e4c8c3f18949 with: format: lcov @@ -148,6 +148,7 @@ jobs: allow-empty: true - name: Upload markdown + if: always() uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 with: name: comment-${{ inputs.checks }} From f6180e1983e1ddfcd22e0d96852b9befca74d3b5 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:28:36 +0100 Subject: [PATCH 41/49] Fixes --- .github/workflows/health.yaml | 8 ++++---- .github/workflows/health_base.yaml | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 8ffd4029..62575ef6 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -179,10 +179,6 @@ jobs: comment-author: github-actions[bot] body-includes: '## PR Health' - - name: Write comment id to file - if: ${{ steps.fc.outputs.comment-id != 0 }} - run: echo ${{ steps.fc.outputs.comment-id }} >> output/commentId - - name: Merge all single comments run: | mkdir output @@ -190,6 +186,10 @@ jobs: cat single-comments/*/*.md >> output/comment.md echo ${{ github.event.number }} > output/issueNumber + - name: Write comment id to file + if: ${{ steps.fc.outputs.comment-id != 0 }} + run: echo ${{ steps.fc.outputs.comment-id }} >> output/commentId + - name: Upload folder with number and markdown uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 with: diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 3f60e14b..3bee02e1 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -139,7 +139,7 @@ jobs: run: cd current_repo/ && dart pub global run firehose:health --checks ${{ inputs.checks }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} --fail_on ${{ inputs.fail_on }} --warn_on ${{ inputs.warn_on }} - name: Upload coverage to Coveralls - if: ${{ always() && inputs.upload_coverage }} + if: ${{ inputs.upload_coverage }} uses: coverallsapp/github-action@3dfc5567390f6fa9267c0ee9c251e4c8c3f18949 with: format: lcov @@ -148,7 +148,7 @@ jobs: allow-empty: true - name: Upload markdown - if: always() + if: success() || failure() uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 with: name: comment-${{ inputs.checks }} From 4fee5d3e141b69a368b2f0f7bd607158a220a0fd Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:34:40 +0100 Subject: [PATCH 42/49] Rename job --- .github/workflows/health.yaml | 12 ++++++------ .github/workflows/health_base.yaml | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 62575ef6..beb33629 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -81,7 +81,7 @@ on: jobs: version: - name: health:version + name: version if: contains(${{ inputs.checks }}, 'version') uses: ./.github/workflows/health_base.yaml with: @@ -93,7 +93,7 @@ jobs: use-flutter: ${{ inputs.use-flutter }} changelog: - name: health:changelog + name: changelog if: contains(${{ inputs.checks }}, 'changelog') uses: ./.github/workflows/health_base.yaml with: @@ -105,7 +105,7 @@ jobs: use-flutter: ${{ inputs.use-flutter }} license: - name: health:license + name: license if: contains(${{ inputs.checks }}, 'license') uses: ./.github/workflows/health_base.yaml with: @@ -117,7 +117,7 @@ jobs: use-flutter: ${{ inputs.use-flutter }} coverage: - name: health:coverage + name: coverage if: contains(${{ inputs.checks }}, 'coverage') uses: ./.github/workflows/health_base.yaml with: @@ -131,7 +131,7 @@ jobs: use-flutter: ${{ inputs.use-flutter }} breaking: - name: health:breaking + name: breaking if: contains(${{ inputs.checks }}, 'breaking') uses: ./.github/workflows/health_base.yaml with: @@ -143,7 +143,7 @@ jobs: use-flutter: ${{ inputs.use-flutter }} do-not-submit: - name: health:do-not-submit + name: do-not-submit if: contains(${{ inputs.checks }}, 'do-not-submit') uses: ./.github/workflows/health_base.yaml with: diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 3bee02e1..04f5c9a8 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -81,6 +81,7 @@ on: jobs: health: + name: run # These permissions are required for us to create comments on PRs. permissions: pull-requests: write From aee37ad94d26a4374b808280d7ad6502a83e75d3 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:45:09 +0100 Subject: [PATCH 43/49] Refactor to single check --- .github/workflows/health.yaml | 12 ++--- .github/workflows/health_base.yaml | 19 ++++---- pkgs/firehose/bin/health.dart | 8 ++-- pkgs/firehose/lib/src/health/health.dart | 56 +++++++++++------------- 4 files changed, 44 insertions(+), 51 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index beb33629..b31eef8e 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -86,7 +86,7 @@ jobs: uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} - checks: version + check: version fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} @@ -98,7 +98,7 @@ jobs: uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} - checks: changelog + check: changelog fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} @@ -110,7 +110,7 @@ jobs: uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} - checks: license + check: license fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} @@ -122,7 +122,7 @@ jobs: uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} - checks: coverage + check: coverage fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} upload_coverage: ${{ inputs.upload_coverage }} @@ -136,7 +136,7 @@ jobs: uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} - checks: breaking + check: breaking fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} @@ -148,7 +148,7 @@ jobs: uses: ./.github/workflows/health_base.yaml with: sdk: ${{ inputs.sdk }} - checks: do-not-submit + check: do-not-submit fail_on: ${{ inputs.fail_on }} warn_on: ${{ inputs.warn_on }} local_debug: ${{ inputs.local_debug }} diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 04f5c9a8..ad0c3268 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -42,11 +42,10 @@ on: default: "stable" required: false type: string - checks: - description: What to check for in the PR health check - any subset of "version,changelog,license,coverage,breaking,do-not-submit" - default: "version,changelog,license,coverage,breaking,do-not-submit" + check: + description: What to check for in the PR health check - any of "version,changelog,license,coverage,breaking,do-not-submit" type: string - required: false + required: true fail_on: description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit" default: "version,changelog,do-not-submit" @@ -97,12 +96,12 @@ jobs: with: ref: ${{ github.event.pull_request.base.ref }} path: base_repo/ - if: contains(inputs.checks, 'coverage') || contains(inputs.checks, 'breaking') + if: ${{ inputs.check == 'coverage' }} || ${{ inputs.check == 'breaking' }} - name: Write comment if not present run: | mkdir -p current_repo/output/ - test -f current_repo/output/comment.md || echo $'The ${{ inputs.checks }} workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md + 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 - uses: subosito/flutter-action@2783a3f08e1baf891508463f8c6653c258246225 if: ${{ inputs.use-flutter }} @@ -116,7 +115,7 @@ jobs: - name: Install coverage run: dart pub global activate coverage - if: contains(inputs.checks, 'coverage') + if: ${{ inputs.check == 'coverage' }} - name: Install firehose run: dart pub global activate firehose @@ -128,7 +127,7 @@ jobs: - name: Install api_tool run: dart pub global activate dart_apitool - if: contains(inputs.checks, 'breaking') + if: ${{ inputs.check == 'breaking' }} - name: Check PR health id: healthstep @@ -137,7 +136,7 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ github.event.number }} PR_LABELS: "${{ join(github.event.pull_request.labels.*.name) }}" - run: cd current_repo/ && dart pub global run firehose:health --checks ${{ inputs.checks }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} --fail_on ${{ inputs.fail_on }} --warn_on ${{ inputs.warn_on }} + run: cd current_repo/ && dart pub global run firehose:health --check ${{ inputs.check }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} --fail_on ${{ inputs.fail_on }} --warn_on ${{ inputs.warn_on }} - name: Upload coverage to Coveralls if: ${{ inputs.upload_coverage }} @@ -152,5 +151,5 @@ jobs: if: success() || failure() uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 with: - name: comment-${{ inputs.checks }} + name: comment-${{ inputs.check }} path: current_repo/output/comment.md \ No newline at end of file diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index 204a3349..19a98d40 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -9,8 +9,8 @@ import 'package:firehose/src/health/health.dart'; void main(List arguments) async { var argParser = ArgParser() - ..addMultiOption( - 'checks', + ..addOption( + 'check', allowed: checkTypes, help: 'Check PR health.', ) @@ -29,7 +29,7 @@ void main(List arguments) async { help: 'Whether to run web tests for coverage', ); var parsedArgs = argParser.parse(arguments); - var checks = parsedArgs['checks'] as List; + var check = parsedArgs['check'] as String; var warnOn = parsedArgs['warn_on'] as List; var failOn = parsedArgs['fail_on'] as List; var coverageWeb = parsedArgs['coverage_web'] as bool; @@ -37,6 +37,6 @@ void main(List arguments) async { throw ArgumentError('The checks for which warnings are displayed and the ' 'checks which lead to failure must be disjoint.'); } - await Health(Directory.current, checks, warnOn, failOn, coverageWeb) + await Health(Directory.current, check, warnOn, failOn, coverageWeb) .healthCheck(); } diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index cfc02bd7..4eb10d33 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -52,14 +52,14 @@ class Health { Health( this.directory, - this.checks, + this.check, this.warnOn, this.failOn, this.coverageweb, ); final github = GithubApi(); - final List checks; + final String check; final List warnOn; final List failOn; final bool coverageweb; @@ -75,29 +75,24 @@ class Health { return; } - print('Start health check for the checks $checks'); - final results = []; - for (final check in checks) { - print('Checking for $check'); - if (!github.prLabels.contains('skip-$check-check')) { - final firstResult = await checkFor(check)(); - final HealthCheckResult finalResult; - if (warnOn.contains(check) && firstResult.severity == Severity.error) { - finalResult = firstResult.withSeverity(Severity.warning); - } else if (failOn.contains(check) && - firstResult.severity == Severity.warning) { - finalResult = firstResult.withSeverity(Severity.error); - } else { - finalResult = firstResult; - } - results.add(finalResult); - print( - '\n\n${finalResult.severity.name.toUpperCase()}: $check done.\n\n'); + print('Start health check for the check $check'); + print('Checking for $check'); + if (!github.prLabels.contains('skip-$check-check')) { + final firstResult = await checkFor(check)(); + final HealthCheckResult finalResult; + if (warnOn.contains(check) && firstResult.severity == Severity.error) { + finalResult = firstResult.withSeverity(Severity.warning); + } else if (failOn.contains(check) && + firstResult.severity == Severity.warning) { + finalResult = firstResult.withSeverity(Severity.error); } else { - print('Skipping $check, as the skip tag is present.'); + finalResult = firstResult; } + await writeInComment(github, finalResult); + print('\n\n${finalResult.severity.name.toUpperCase()}: $check done.\n\n'); + } else { + print('Skipping $check, as the skip tag is present.'); } - await writeInComment(github, results); } String tagFor(String checkType) => switch (checkType) { @@ -326,11 +321,10 @@ This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test- } Future writeInComment( - GithubApi github, List results) async { + GithubApi github, HealthCheckResult result) async { await saveExistingCommentId(github); - var summary = ''; - final commentText = - results.where((result) => result.markdown != null).map((result) { + final String markdownSummary; + if (result.markdown != null) { var markdown = result.markdown; var isWorseThanInfo = result.severity.index >= Severity.warning.index; var s = ''' @@ -345,10 +339,11 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r '''; - return '${tagFor(result.name)} ${result.severity.emoji}\n\n$s'; - }).join('\n'); + markdownSummary = '${tagFor(result.name)} ${result.severity.emoji}\n\n$s'; + } else { + markdownSummary = ''; + } - final markdownSummary = summary + commentText; github.appendStepSummary(markdownSummary); var commentFile = File('./output/comment.md'); @@ -356,8 +351,7 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r await commentFile.create(recursive: true); await commentFile.writeAsString(markdownSummary); - if (results.any((result) => result.severity == Severity.error) && - exitCode == 0) { + if (result.severity == Severity.error && exitCode == 0) { exitCode = 1; } } From 70009342d5e8108f61da9ccc93f8039914dea6df Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:49:15 +0100 Subject: [PATCH 44/49] Don't fail on changelog --- .github/workflows/health_internal.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index 79fbe9e6..3892ba32 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -12,4 +12,6 @@ jobs: local_debug: true coverage_web: false upload_coverage: false - checks: version,changelog,license,coverage,breaking,do-not-submit \ No newline at end of file + checks: version,changelog,license,coverage,breaking,do-not-submit + fail_on: version,do-not-submit + warn_on: changelog,license,coverage,breaking \ No newline at end of file From 4e84c0c040e71618133c24936a33248402495f8c Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:52:44 +0100 Subject: [PATCH 45/49] Cleanups --- .github/workflows/health_internal.yaml | 4 ++-- pkgs/firehose/lib/src/health/health.dart | 28 ------------------------ 2 files changed, 2 insertions(+), 30 deletions(-) diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index 3892ba32..7c846d51 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -13,5 +13,5 @@ jobs: coverage_web: false upload_coverage: false checks: version,changelog,license,coverage,breaking,do-not-submit - fail_on: version,do-not-submit - warn_on: changelog,license,coverage,breaking \ No newline at end of file + fail_on: version,changelog,do-not-submit + warn_on: license,coverage,breaking \ No newline at end of file diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index 4eb10d33..5dab5e4d 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -22,8 +22,6 @@ import 'license.dart'; const String _botSuffix = '[bot]'; -const String _githubActionsUser = 'github-actions[bot]'; - const String _publishBotTag2 = '### Package publish validation'; const String _licenseBotTag = '### License Headers'; @@ -36,8 +34,6 @@ const String _coverageBotTag = '### Coverage'; const String _breakingBotTag = '### Breaking changes'; -const String _prHealthTag = '## PR Health'; - const checkTypes = [ 'version', 'license', @@ -322,7 +318,6 @@ This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test- Future writeInComment( GithubApi github, HealthCheckResult result) async { - await saveExistingCommentId(github); final String markdownSummary; if (result.markdown != null) { var markdown = result.markdown; @@ -356,21 +351,6 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r } } - Future saveExistingCommentId(GithubApi github) async { - var existingComment = await allowFailure( - github.findCommentId(user: _githubActionsUser, searchTerm: _prHealthTag), - logError: print, - ); - - if (existingComment != null) { - var idFile = File('./output/commentId'); - print(''' - Saving existing comment id $existingComment to file ${idFile.path}'''); - await idFile.create(recursive: true); - await idFile.writeAsString(existingComment.id.toString()); - } - } - List packagesContaining(List filesInPR) { var files = filesInPR.where((element) => element.status.isRelevant); final repo = Repository(); @@ -383,14 +363,6 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r } } -Version getNewVersion(BreakingLevel level, Version oldVersion) { - return switch (level) { - BreakingLevel.none => oldVersion, - BreakingLevel.nonBreaking => oldVersion.nextMinor, - BreakingLevel.breaking => oldVersion.nextBreaking, - }; -} - enum BreakingLevel { none('None'), nonBreaking('Non-Breaking'), From a5c3912f91576954bb02a4cead10bec223f18685 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 12:53:26 +0100 Subject: [PATCH 46/49] Add changelog --- pkgs/firehose/CHANGELOG.md | 4 ++++ pkgs/firehose/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md index 94b3dd43..23613cda 100644 --- a/pkgs/firehose/CHANGELOG.md +++ b/pkgs/firehose/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.5.0 + +- Split health checks in individual workflows. + ## 0.4.2 - Get needed version from `dart_apitool` in PR health checks. diff --git a/pkgs/firehose/pubspec.yaml b/pkgs/firehose/pubspec.yaml index 404af5e8..57e90f3f 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.4.2 +version: 0.5.0 repository: https://github.com/dart-lang/ecosystem/tree/main/pkgs/firehose environment: From ed74b975b779bfb7dfd1abe6e28f48ac71ef7e89 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 13:03:05 +0100 Subject: [PATCH 47/49] Remove names --- .github/workflows/health.yaml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index b31eef8e..95e27f42 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -81,7 +81,6 @@ on: jobs: version: - name: version if: contains(${{ inputs.checks }}, 'version') uses: ./.github/workflows/health_base.yaml with: @@ -93,7 +92,6 @@ jobs: use-flutter: ${{ inputs.use-flutter }} changelog: - name: changelog if: contains(${{ inputs.checks }}, 'changelog') uses: ./.github/workflows/health_base.yaml with: @@ -105,7 +103,6 @@ jobs: use-flutter: ${{ inputs.use-flutter }} license: - name: license if: contains(${{ inputs.checks }}, 'license') uses: ./.github/workflows/health_base.yaml with: @@ -117,7 +114,6 @@ jobs: use-flutter: ${{ inputs.use-flutter }} coverage: - name: coverage if: contains(${{ inputs.checks }}, 'coverage') uses: ./.github/workflows/health_base.yaml with: @@ -131,7 +127,6 @@ jobs: use-flutter: ${{ inputs.use-flutter }} breaking: - name: breaking if: contains(${{ inputs.checks }}, 'breaking') uses: ./.github/workflows/health_base.yaml with: @@ -143,7 +138,6 @@ jobs: use-flutter: ${{ inputs.use-flutter }} do-not-submit: - name: do-not-submit if: contains(${{ inputs.checks }}, 'do-not-submit') uses: ./.github/workflows/health_base.yaml with: From 3e0fb4ac1e7835c2785fb8c08dd29e6fbe34a9e1 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 14:30:34 +0100 Subject: [PATCH 48/49] Fix documentation --- .github/workflows/health.yaml | 27 ++++++++++++++++----------- .github/workflows/health_base.yaml | 29 +---------------------------- 2 files changed, 17 insertions(+), 39 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 95e27f42..d2cf26e1 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -12,17 +12,9 @@ name: Health # jobs: # health: # uses: dart-lang/ecosystem/.github/workflows/health.yaml@main -# with: -# coverage_web: true #If the coverage should run browser tests - -# Callers may optionally specify the version of the SDK to use when running the -# health check. This can be useful if your package has a very recent minimum SDK -# constraint. This is done via the `sdk` input parameter. Note that this -# parameter is not required; it defaults to `stable` - using the most recent -# stable release of the Dart SDK. -# -# The checks can also be restricted to any subset of version, changelog, and license, -# if needed. + + +# Or with options: # # jobs: # health: @@ -30,10 +22,21 @@ name: Health # with: # sdk: beta # checks: "version,changelog,license,coverage,breaking,do-not-submit" +# fail_on: "version,changelog,do-not-submit" +# warn_on: "license,coverage,breaking" +# coverage_web: false +# upload_coverage: false +# use-flutter: true + on: workflow_call: inputs: + # Callers may optionally specify the version of the SDK to use when running the + # health check. This can be useful if your package has a very recent minimum SDK + # constraint. This is done via the `sdk` input parameter. Note that this + # parameter is not required; it defaults to `stable` - using the most recent + # stable release of the Dart SDK. sdk: description: >- The channel, or a specific version from a channel, to install @@ -42,6 +45,8 @@ on: default: "stable" required: false type: string + # Restrict the checks to any subset of version, changelog, and license if + # needed. checks: description: What to check for in the PR health check - any subset of "version,changelog,license,coverage,breaking,do-not-submit" default: "version,changelog,license,coverage,breaking,do-not-submit" diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index ad0c3268..57e0f3bc 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -2,34 +2,7 @@ name: Health:Base -# Callers of this workflow should use it as follows: -# -# name: Health -# on: -# pull_request: -# branches: [ main ] -# types: [opened, synchronize, reopened, labeled, unlabeled] -# jobs: -# health: -# uses: dart-lang/ecosystem/.github/workflows/health.yaml@main -# with: -# coverage_web: true #If the coverage should run browser tests - -# Callers may optionally specify the version of the SDK to use when running the -# health check. This can be useful if your package has a very recent minimum SDK -# constraint. This is done via the `sdk` input parameter. Note that this -# parameter is not required; it defaults to `stable` - using the most recent -# stable release of the Dart SDK. -# -# The checks can also be restricted to any subset of version, changelog, and license, -# if needed. -# -# jobs: -# health: -# uses: dart-lang/ecosystem/.github/workflows/health.yaml@main -# with: -# sdk: beta -# checks: "version,changelog,license,coverage,breaking,do-not-submit" +# The workflow doing the checks for `health.yaml`. Not meant to be used externally. on: workflow_call: From 0e21976f8c958228438300241e4ab80bb2466914 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 5 Jan 2024 14:31:43 +0100 Subject: [PATCH 49/49] Add newlines --- .github/workflows/health_base.yaml | 2 +- .github/workflows/health_internal.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml index 57e0f3bc..8870ec0c 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -125,4 +125,4 @@ jobs: uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 with: name: comment-${{ inputs.check }} - path: current_repo/output/comment.md \ No newline at end of file + path: current_repo/output/comment.md diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index 7c846d51..cf46b50f 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -14,4 +14,4 @@ jobs: upload_coverage: false checks: version,changelog,license,coverage,breaking,do-not-submit fail_on: version,changelog,do-not-submit - warn_on: license,coverage,breaking \ No newline at end of file + warn_on: license,coverage,breaking