Skip to content

Commit

Permalink
[tool] Allow blank lines in federated package changes (#8468)
Browse files Browse the repository at this point in the history
Currently the special-casing for allowing comment-only changes to other packages in a plugin group as part of a single PR, which is necessary for some changes to add `// ignore` directives, allows comment lines, but not blank lines. Blank lines should be allowed since they are safe, and are expected for certain changes (notably, temporary file-level ignores).
  • Loading branch information
stuartmorgan authored Jan 21, 2025
1 parent e8f1f63 commit 3d3ab7b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
5 changes: 3 additions & 2 deletions script/tool/lib/src/federation_safety_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -212,18 +212,19 @@ class FederationSafetyCheckCommand extends PackageLoopingCommand {
Future<bool> _changeIsCommentOnly(
GitVersionFinder git, String repoPath) async {
final List<String> diff = await git.getDiffContents(targetPath: repoPath);
final RegExp changeLine = RegExp(r'^[+-] ');
final RegExp changeLine = RegExp(r'^[+-]');
// This will not catch /**/-style comments, but false negatives are fine
// (and in practice, we almost never use that comment style in Dart code).
final RegExp commentLine = RegExp(r'^[+-]\s*//');
final RegExp blankLine = RegExp(r'^[+-]\s*$');
bool foundComment = false;
for (final String line in diff) {
if (!changeLine.hasMatch(line) ||
line.startsWith('--- ') ||
line.startsWith('+++ ')) {
continue;
}
if (!commentLine.hasMatch(line)) {
if (!(commentLine.hasMatch(line) || blankLine.hasMatch(line))) {
return false;
}
foundComment = true;
Expand Down
4 changes: 3 additions & 1 deletion script/tool/test/federation_safety_check_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -446,10 +446,12 @@ diff --git a/packages/foo/foo_bar/lib/foo.dart b/packages/foo/foo_bar/lib/foo.da
index abc123..def456 100644
--- a/packages/foo/foo_bar/lib/foo.dart
+++ b/packages/foo/foo_bar/lib/foo.dart
@@ -51,6 +51,7 @@ Future<bool> launchUrl(
@@ -51,6 +51,9 @@ Future<bool> launchUrl(
}
void foo() {
+ // blank lines should also be allowed as part of comment changes.
+
+ // ignore: exhaustive_cases
switch(a_foo) {
case a:
Expand Down

0 comments on commit 3d3ab7b

Please sign in to comment.