Skip to content

Commit

Permalink
Check for DO_NOT_SUBMIT strings. (#178)
Browse files Browse the repository at this point in the history
* 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 ada4234.

* Fix call

* Remove double
  • Loading branch information
mosuem authored Sep 28, 2023
1 parent bc2dd27 commit 1154183
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 11 deletions.
7 changes: 3 additions & 4 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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 }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/health_internal.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions pkgs/firehose/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
9 changes: 8 additions & 1 deletion pkgs/firehose/bin/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@ void main(List<String> 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(
Expand Down
41 changes: 36 additions & 5 deletions pkgs/firehose/lib/src/health/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -238,6 +243,31 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst
);
}

Future<HealthCheckResult> 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<HealthCheckResult> coverageCheck(
Github github,
bool coverageWeb,
Expand Down Expand Up @@ -266,7 +296,8 @@ This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test-
Github github,
List<HealthCheckResult> 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 = '''
Expand All @@ -281,7 +312,7 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r
</details>
''';
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';
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 1154183

Please sign in to comment.