Skip to content

Commit 842e482

Browse files
chunhtaichunhtai
committed
update
1 parent a260bbe commit 842e482

File tree

4 files changed

+96
-78
lines changed

4 files changed

+96
-78
lines changed

‎script/tool/lib/src/common/package_state_utils.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ Future<PackageChangeState> checkPackageChangeState(
8080
continue;
8181
}
8282

83+
if (package.parseCiConfig()?.isBatchRelease ?? false) {
84+
if (components.first == 'pending_changelogs') {
85+
hasChangelogChange = true;
86+
continue;
87+
}
88+
}
89+
8390
if (!needsVersionChange) {
8491
// Developer-only changes don't need version changes or changelog changes.
8592
if (await _isDevChange(components, git: git, repoPath: path)) {

‎script/tool/lib/src/version_check_command.dart‎

Lines changed: 50 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,29 @@ class VersionCheckCommand extends PackageLoopingCommand {
208208
// change back to main branch. We can proceed with ragular version check.
209209
final bool hasPostReleaseLabel =
210210
_prLabels.contains('post-release-${pubspec.name}');
211+
bool versionChanged;
212+
211213
if (usesBatchRelease && !hasPostReleaseLabel) {
212214
final List<String> changedFiles =
213215
await _gitVersionFinder.getChangedFiles();
214216
// For batch release, we only check pending changelog files.
215-
errors.addAll(await _validatePendingChangelogs(package, changedFiles));
216-
// The changelog and version should not be updated directly.
217+
final List<PendingChangelogEntry> allChangelogs =
218+
<PendingChangelogEntry>[];
219+
try {
220+
allChangelogs.addAll(package.getPendingChangelogs());
221+
} on FormatException catch (e) {
222+
errors.add(e.message);
223+
return PackageResult.fail(errors);
224+
}
225+
226+
final List<PendingChangelogEntry> newEntries = allChangelogs
227+
.where((PendingChangelogEntry entry) =>
228+
changedFiles.contains(entry.file.path))
229+
.toList();
230+
versionChanged = newEntries.any(
231+
(PendingChangelogEntry entry) => entry.version != VersionChange.skip);
232+
233+
// The changelog.md and pubspec.yaml's version should not be updated directly.
217234
if (changedFiles.contains(package.changelogFile.path)) {
218235
errors.add('CHANGELOG.md changed');
219236
}
@@ -223,7 +240,6 @@ class VersionCheckCommand extends PackageLoopingCommand {
223240
}
224241
}
225242
} else {
226-
bool versionChanged;
227243
switch (versionState) {
228244
case _CurrentVersionState.unchanged:
229245
versionChanged = false;
@@ -243,16 +259,16 @@ class VersionCheckCommand extends PackageLoopingCommand {
243259
pubspec: pubspec, pubspecVersionState: versionState))) {
244260
errors.add('CHANGELOG.md failed validation.');
245261
}
262+
}
246263

247-
// If there are no other issues, make sure that there isn't a missing
248-
// change to the version and/or CHANGELOG.
249-
if (getBoolArg(_checkForMissingChanges) &&
250-
!versionChanged &&
251-
errors.isEmpty) {
252-
final String? error = await _checkForMissingChangeError(package);
253-
if (error != null) {
254-
errors.add(error);
255-
}
264+
// If there are no other issues, make sure that there isn't a missing
265+
// change to the version and/or CHANGELOG.
266+
if (getBoolArg(_checkForMissingChanges) &&
267+
!versionChanged &&
268+
errors.isEmpty) {
269+
final String? error = await _checkForMissingChangeError(package);
270+
if (error != null) {
271+
errors.add(error);
256272
}
257273
}
258274

@@ -594,14 +610,28 @@ ${indentation}The first version listed in CHANGELOG.md is $fromChangeLog.
594610
'"$_missingChangelogChangeOverrideLabel" label.');
595611
} else {
596612
missingChangelogChange = true;
597-
printError('No CHANGELOG change found.\n'
598-
'If this PR needs an exemption from the standard policy of listing '
599-
'all changes in the CHANGELOG,\n'
600-
'comment in the PR to explain why the PR is exempt, and add (or '
601-
'ask your reviewer to add) the\n'
602-
'"$_missingChangelogChangeOverrideLabel" label.\n'
603-
'Otherwise, please add a NEXT entry in the CHANGELOG as described in '
604-
'the contributing guide.\n');
613+
final bool isBatchRelease =
614+
package.parseCiConfig()?.isBatchRelease ?? false;
615+
if (isBatchRelease) {
616+
printError(
617+
'No new changelog files found in the pending_changelogs folder.\n'
618+
'If this PR needs an exemption from the standard policy of listing '
619+
'all changes in the CHANGELOG,\n'
620+
'comment in the PR to explain why the PR is exempt, and add (or '
621+
'ask your reviewer to add) the\n'
622+
'"$_missingChangelogChangeOverrideLabel" label.\n'
623+
'Otherwise, please add a NEXT entry in the CHANGELOG as described in '
624+
'the contributing guide.\n');
625+
} else {
626+
printError('No CHANGELOG change found.\n'
627+
'If this PR needs an exemption from the standard policy of listing '
628+
'all changes in the CHANGELOG,\n'
629+
'comment in the PR to explain why the PR is exempt, and add (or '
630+
'ask your reviewer to add) the\n'
631+
'"$_missingChangelogChangeOverrideLabel" label.\n'
632+
'Otherwise, please add a NEXT entry in the CHANGELOG as described in '
633+
'the contributing guide.\n');
634+
}
605635
}
606636
}
607637

@@ -621,59 +651,4 @@ ${indentation}The first version listed in CHANGELOG.md is $fromChangeLog.
621651

622652
return null;
623653
}
624-
625-
Future<List<String>> _validatePendingChangelogs(
626-
RepositoryPackage package, List<String> changedPaths) async {
627-
final List<String> errors = <String>[];
628-
List<PendingChangelogEntry> allChangelogs = <PendingChangelogEntry>[];
629-
try {
630-
allChangelogs = package.getPendingChangelogs();
631-
} on FormatException catch (e) {
632-
errors.add(e.message);
633-
return errors;
634-
}
635-
636-
final List<PendingChangelogEntry> newEntries = allChangelogs
637-
.where((PendingChangelogEntry entry) =>
638-
changedPaths.contains(entry.file.path))
639-
.toList();
640-
641-
if (newEntries.isEmpty) {
642-
if (_prLabels.contains(_missingChangelogChangeOverrideLabel)) {
643-
logWarning('Ignoring lack of changelog update due to the '
644-
'"$_missingChangelogChangeOverrideLabel" label.');
645-
} else {
646-
errors.add('Missing CHANGELOG file');
647-
printError(
648-
'No new changelog files found in the pending_changelogs folder.\n'
649-
'If this PR needs an exemption from the standard policy of listing '
650-
'all changes in the CHANGELOG,\n'
651-
'comment in the PR to explain why the PR is exempt, and add (or '
652-
'ask your reviewer to add) the\n'
653-
'"$_missingChangelogChangeOverrideLabel" label.\n'
654-
'Otherwise, please add a NEXT entry in the CHANGELOG as described in '
655-
'the contributing guide.\n');
656-
}
657-
} else {
658-
final bool needsOverride = newEntries.every(
659-
(PendingChangelogEntry entry) => entry.version == VersionChange.skip);
660-
if (needsOverride) {
661-
if (_prLabels.contains(_missingVersionChangeOverrideLabel)) {
662-
logWarning('Ignoring lack of version change due to the '
663-
'"$_missingVersionChangeOverrideLabel" label.');
664-
} else {
665-
printError(
666-
'No version change found, but the change to this package could '
667-
'not be verified to be exempt\n'
668-
'from version changes according to repository policy.\n'
669-
'If this is a false positive, please comment in '
670-
'the PR to explain why the PR\n'
671-
'is exempt, and add (or ask your reviewer to add) the '
672-
'"$_missingVersionChangeOverrideLabel" label.\n');
673-
}
674-
errors.add('Missing version change');
675-
}
676-
}
677-
return errors;
678-
}
679654
}

‎script/tool/test/common/package_state_utils_test.dart‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,26 @@ void main() {
498498
expect(state.needsVersionChange, true);
499499
expect(state.needsChangelogChange, true);
500500
});
501+
502+
test('detects pending changelog changes for batch release', () async {
503+
final RepositoryPackage package =
504+
createFakePackage('a_package', packagesDir);
505+
package.ciConfigFile.writeAsStringSync('''
506+
release:
507+
batch: true
508+
''');
509+
510+
const List<String> changedFiles = <String>[
511+
'packages/a_package/pending_changelogs/some_change.yaml',
512+
];
513+
514+
final PackageChangeState state = await checkPackageChangeState(package,
515+
changedPaths: changedFiles,
516+
relativePackagePath: 'packages/a_package/');
517+
518+
expect(state.hasChanges, true);
519+
expect(state.hasChangelogChange, true);
520+
});
501521
});
502522
}
503523

‎script/tool/test/version_check_command_test.dart‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -345,22 +345,37 @@ release:
345345
);
346346
});
347347

348-
test('fails for batch release package with no new changelog', () async {
348+
test(
349+
'fails for batch release package with no new changelog and post-release label',
350+
() async {
349351
final RepositoryPackage package =
350352
createFakePackage('package', packagesDir, version: '1.0.0');
351353
package.ciConfigFile.writeAsStringSync('''
352354
release:
353355
batch: true
354356
''');
357+
// Simulate a code change
358+
package.libDirectory
359+
.childFile('foo.dart')
360+
.writeAsStringSync('void foo() {}');
361+
// Create the pending_changelogs directory so we don't fail on that check.
362+
package.directory.childDirectory('pending_changelogs').createSync();
363+
355364
gitProcessRunner.mockProcessesForExecutable['git-diff'] =
356365
<FakeProcessInfo>[
357-
FakeProcessInfo(MockProcess(stdout: '')),
366+
FakeProcessInfo(MockProcess(stdout: 'packages/package/lib/foo.dart')),
367+
];
368+
gitProcessRunner.mockProcessesForExecutable['git-show'] =
369+
<FakeProcessInfo>[
370+
FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')),
358371
];
359372

360373
Error? commandError;
361374
final List<String> output = await runCapturingPrint(runner, <String>[
362375
'version-check',
363376
'--base-sha=main',
377+
'--check-for-missing-changes',
378+
'--pr-labels=post-release-package',
364379
], errorHandler: (Error e) {
365380
commandError = e;
366381
});
@@ -369,7 +384,8 @@ release:
369384
expect(
370385
output,
371386
containsAllInOrder(<Matcher>[
372-
contains('No pending_changelogs folder found for package'),
387+
contains(
388+
'No new changelog files found in the pending_changelogs folder.'),
373389
]));
374390
});
375391

0 commit comments

Comments
 (0)