From 11541831eacde27992140b80bf12e27c2650971a Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 28 Sep 2023 15:00:54 +0200 Subject: [PATCH] Check for `DO_NOT_SUBMIT` strings. (#178) * Add api tool call for testing * Change tag * Add debugging log * look in each package * debug * take subpackage * Run in current * add debug * debug * Add argument for testing * Switch to local copy * Add pub get to apitool clone * move dart install * Revert changes * Fix ref * dart fix * Switch to main branch * use new * use report file * Add test argument * Organize imports * Nicer output * Fix version * Really fix * fix path * Make nicer * Add severity * Even nicer * Fix layout * Make bold * Capitalize * Really bolden * Enable all checks * Rev version * Make nullsafer * add DO_NOT_SUBMIT * Add `DO_NOT_SUBMIT` check * Add do-not-submit to workflow * Add print statements * Add print statements - fixed * Remove empty file * Remove empty file * mask donotsubmits * Do not continue on error * mask more donotsubmits * Changes as per review * Revert "Add test argument" This reverts commit ada423495503d0f90d5499af1994eee49f0c87a1. * Fix call * Remove double --- .github/workflows/health.yaml | 7 ++-- .github/workflows/health_internal.yaml | 2 +- pkgs/firehose/CHANGELOG.md | 1 + pkgs/firehose/bin/health.dart | 9 +++++- pkgs/firehose/lib/src/health/health.dart | 41 +++++++++++++++++++++--- 5 files changed, 49 insertions(+), 11 deletions(-) diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index b9648164..34ec93cb 100644 --- a/.github/workflows/health.yaml +++ b/.github/workflows/health.yaml @@ -29,7 +29,7 @@ name: Health # uses: dart-lang/ecosystem/.github/workflows/health.yaml@main # with: # sdk: beta -# checks: "version,changelog,license,coverage,breaking" +# checks: "version,changelog,license,coverage,breaking,do-not-submit" on: workflow_call: @@ -43,8 +43,8 @@ on: required: false type: string checks: - description: What to check for in the PR health check - any subset of "version,changelog,license,coverage,breaking" - default: "version,changelog,license,coverage,breaking" + 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 local_debug: @@ -101,7 +101,6 @@ jobs: - name: Check PR health if: ${{ github.event_name == 'pull_request' }} - continue-on-error: true env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ github.event.number }} diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index 826bc488..aae8ba10 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 + checks: version,changelog,license,coverage,breaking,do-not-submit diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md index 2efbc418..1c35aa89 100644 --- a/pkgs/firehose/CHANGELOG.md +++ b/pkgs/firehose/CHANGELOG.md @@ -1,6 +1,7 @@ ## 0.3.31 - Add PR Health checks for breaking changes. +- Add PR Health checks for `DO_NOT${'_'}SUBMIT` strings. ## 0.3.30 diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index e87270b1..1f6ab49a 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -11,7 +11,14 @@ void main(List arguments) async { var argParser = ArgParser() ..addMultiOption( 'checks', - allowed: ['version', 'license', 'changelog', 'coverage', 'breaking'], + allowed: [ + 'version', + 'license', + 'changelog', + 'coverage', + 'breaking', + 'do-not-submit', + ], help: 'Check PR health.', ) ..addFlag( diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index bb6487a0..c49ba5b7 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -30,6 +30,8 @@ const String _licenseBotTag = '### License Headers'; const String _changelogBotTag = '### Changelog Entry'; +const String _doNotSubmitBotTag = '### Do Not Submit'; + const String _coverageBotTag = '### Coverage'; const String _breakingBotTag = '### Breaking changes'; @@ -72,6 +74,9 @@ class Health { 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, ]; var checked = @@ -238,6 +243,31 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst ); } + Future doNotSubmitCheck(Github github) async { + final files = await github.listFilesForPR(); + print('Checking for DO_NOT${'_'}SUBMIT strings: $files'); + var filesWithDNS = files + .where((file) => + ![FileStatus.removed, FileStatus.unchanged].contains(file.status)) + .where((file) => File(file.relativePath) + .readAsStringSync() + .contains('DO_NOT${'_'}SUBMIT')) + .toList(); + print('Found files with DO_NOT_${'SUBMIT'}: $filesWithDNS'); + final markdownResult = ''' +| Files with `DO_NOT_${'SUBMIT'}` | +| :--- | +${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} +'''; + + return HealthCheckResult( + 'do-not-submit', + _doNotSubmitBotTag, + filesWithDNS.isNotEmpty ? Severity.error : Severity.success, + filesWithDNS.isNotEmpty ? markdownResult : null, + ); + } + Future coverageCheck( Github github, bool coverageWeb, @@ -266,7 +296,8 @@ This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test- Github github, List results, ) async { - var commentText = results.map((result) { + var commentText = + results.where((result) => result.markdown != null).map((result) { var markdown = result.markdown; var isWorseThanInfo = result.severity.index >= Severity.warning.index; var s = ''' @@ -281,7 +312,7 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r '''; - return '${result.tag} ${result.severity.emoji}\n\n$s'; + return '${result.title} ${result.severity.emoji}\n\n$s'; }).join('\n'); var summary = '$_prHealthTag\n\n$commentText'; @@ -340,11 +371,11 @@ enum BreakingLevel { class HealthCheckResult { final String name; - final String tag; + final String title; final Severity severity; - final String markdown; + final String? markdown; - HealthCheckResult(this.name, this.tag, this.severity, this.markdown); + HealthCheckResult(this.name, this.title, this.severity, this.markdown); } class BreakingChange {