diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 798d1cdf..b9648164 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" +# checks: "version,changelog,license,coverage,breaking" 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" - default: "version,changelog,license,coverage" + description: What to check for in the PR health check - any subset of "version,changelog,license,coverage,breaking" + default: "version,changelog,license,coverage,breaking" type: string required: false local_debug: @@ -92,6 +92,9 @@ jobs: run: dart pub global activate firehose if: ${{ !inputs.local_debug }} + - name: Install api_tool + run: dart pub global activate --source git https://github.com/bmw-tech/dart_apitool + - name: Install local firehose run: dart pub global activate --source path current_repo/pkgs/firehose/ if: ${{ inputs.local_debug }} diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml index 036e67a0..826bc488 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -11,3 +11,5 @@ jobs: with: local_debug: true coverage_web: false + upload_coverage: false + checks: version,changelog,license,coverage,breaking diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md index f3d87b93..2efbc418 100644 --- a/pkgs/firehose/CHANGELOG.md +++ b/pkgs/firehose/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.3.31 + +- Add PR Health checks for breaking changes. + ## 0.3.30 - Improve support for `-dev` and `-wip` package versions. diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart index 25b80524..e87270b1 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -11,7 +11,7 @@ void main(List arguments) async { var argParser = ArgParser() ..addMultiOption( 'checks', - allowed: ['version', 'license', 'changelog', 'coverage'], + allowed: ['version', 'license', 'changelog', 'coverage', 'breaking'], help: 'Check PR health.', ) ..addFlag( diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart index dcd24372..bb6487a0 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -4,13 +4,17 @@ // ignore_for_file: always_declare_return_types +import 'dart:convert'; import 'dart:io'; import 'dart:math'; import 'package:collection/collection.dart'; +import 'package:path/path.dart' as path; +import 'package:pub_semver/pub_semver.dart'; import '../../firehose.dart'; import '../github.dart'; +import '../repo.dart'; import '../utils.dart'; import 'changelog.dart'; import 'coverage.dart'; @@ -28,6 +32,8 @@ const String _changelogBotTag = '### Changelog Entry'; const String _coverageBotTag = '### Coverage'; +const String _breakingBotTag = '### Breaking changes'; + const String _prHealthTag = '## PR Health'; class Health { @@ -63,6 +69,9 @@ class Health { if (args.contains('coverage') && !github.prLabels.contains('skip-coverage-check')) (Github github) => coverageCheck(github, coverageweb), + if (args.contains('breaking') && + !github.prLabels.contains('skip-breaking-check')) + breakingCheck, ]; var checked = @@ -92,6 +101,80 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati ); } + Future breakingCheck(Github github) async { + final repo = Repository(); + final packages = repo.locatePackages(); + var changeForPackage = {}; + var baseDirectory = Directory('../base_repo'); + for (var package in packages) { + var currentPath = + path.relative(package.directory.path, from: Directory.current.path); + var basePackage = path.relative( + path.join(baseDirectory.absolute.path, currentPath), + from: currentPath, + ); + print('Look for changes in $currentPath with base $basePackage'); + var runApiTool = Process.runSync( + 'dart', + [ + ...['pub', 'global', 'run'], + 'dart_apitool:main', + 'diff', + ...['--old', basePackage], + ...['--new', '.'], + ...['--report-format', 'json'], + ...['--report-file-path', 'report.json'], + ], + workingDirectory: currentPath, + ); + print(runApiTool.stderr); + print(runApiTool.stdout); + + final reportFile = File(path.join(currentPath, 'report.json')); + var fullReportString = reportFile.readAsStringSync(); + var decoded = jsonDecode(fullReportString) as Map; + var report = decoded['report'] as Map; + + var formattedChanges = const JsonEncoder.withIndent(' ').convert(report); + print('Breaking change report:\n$formattedChanges'); + + BreakingLevel breakingLevel; + if ((report['noChangesDetected'] as bool?) ?? false) { + breakingLevel = BreakingLevel.none; + } else { + if ((report['breakingChanges'] as Map? ?? {}).isNotEmpty) { + breakingLevel = BreakingLevel.breaking; + } else if ((report['nonBreakingChanges'] as Map? ?? {}).isNotEmpty) { + breakingLevel = BreakingLevel.nonBreaking; + } else { + breakingLevel = BreakingLevel.none; + } + } + + var oldPackage = Package( + Directory(path.join(baseDirectory.path, currentPath)), + package.repository, + ); + changeForPackage[package] = BreakingChange( + level: breakingLevel, + oldVersion: oldPackage.version!, + newVersion: package.version!, + ); + } + return HealthCheckResult( + 'breaking', + _breakingBotTag, + changeForPackage.values.any((element) => !element.versionIsFine) + ? Severity.warning + : Severity.info, + ''' +| Package | Change | Current Version | New Version | Needed Version | Looking good? | +| :--- | :--- | ---: | ---: | ---: | ---: | +${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()}|').join('\n')} +''', + ); + } + Future licenseCheck(Github github) async { var files = await github.listFilesForPR(); var allFilePaths = await getFilesWithoutLicenses(Directory.current); @@ -237,6 +320,24 @@ Saving existing comment id $existingCommentId to file ${idFile.path}'''); } } +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'), + breaking('Breaking'); + + final String name; + + const BreakingLevel(this.name); +} + class HealthCheckResult { final String name; final String tag; @@ -245,3 +346,27 @@ class HealthCheckResult { HealthCheckResult(this.name, this.tag, this.severity, this.markdown); } + +class BreakingChange { + final BreakingLevel level; + final Version oldVersion; + final Version newVersion; + + BreakingChange({ + required this.level, + required this.oldVersion, + required this.newVersion, + }); + + Version get suggestedNewVersion => getNewVersion(level, oldVersion); + + bool get versionIsFine => newVersion == suggestedNewVersion; + + String toMarkdownRow() => [ + level.name, + oldVersion, + newVersion, + versionIsFine ? suggestedNewVersion : '**$suggestedNewVersion**', + versionIsFine ? ':heavy_check_mark:' : ':warning:' + ].map((e) => e.toString()).join('|'); +} diff --git a/pkgs/firehose/lib/src/repo.dart b/pkgs/firehose/lib/src/repo.dart index 8336a5a5..0ca96acb 100644 --- a/pkgs/firehose/lib/src/repo.dart +++ b/pkgs/firehose/lib/src/repo.dart @@ -5,6 +5,7 @@ import 'dart:io'; import 'package:path/path.dart' as path; +import 'package:pub_semver/pub_semver.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; import 'package:yaml/yaml.dart' as yaml; @@ -99,7 +100,7 @@ class Package { String get name => pubspec.name; - String? get version => pubspec.version?.toString(); + Version? get version => pubspec.version; @override String toString() { diff --git a/pkgs/firehose/pubspec.yaml b/pkgs/firehose/pubspec.yaml index 3f16ad1c..a2caf145 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.3.30 +version: 0.3.31 repository: https://github.com/dart-lang/ecosystem/tree/main/pkgs/firehose environment: diff --git a/pkgs/firehose/test/repo_test.dart b/pkgs/firehose/test/repo_test.dart index 283a4e3b..34e93aea 100644 --- a/pkgs/firehose/test/repo_test.dart +++ b/pkgs/firehose/test/repo_test.dart @@ -34,7 +34,7 @@ void main() { final queryParams = releaseUri.queryParameters; expect(queryParams['tag'], packages.calculateRepoTag(package)); expect(queryParams['title'], - allOf(contains(package.name), contains(package.version))); + allOf(contains(package.name), contains(package.version.toString()))); expect(queryParams['body'], package.changelog.describeLatestChanges); }); });