Skip to content

Commit

Permalink
Revert "Enable lints library_private_types_in_public_api, `sort_chi…
Browse files Browse the repository at this point in the history
…ld_properties_last` and `use_key_in_widget_constructors`" (#5691)

This reverts commit 5d92a47.

This includes a fix for a latent bug in the version-check repo tooling command that caused it to fail when reverting a package that previously had a NEXT section, so that tests will pass.
  • Loading branch information
stuartmorgan authored May 10, 2022
1 parent 5d92a47 commit 1a124b1
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 15 deletions.
6 changes: 5 additions & 1 deletion script/tool/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## NEXT

- Fixes changelog validation when reverting to a `NEXT` state.

## 0.8.5

- Updates `test` to inculde the Dart unit tests of examples, if any.
Expand Down Expand Up @@ -322,7 +326,7 @@ and `firebase-test-lab`.

## v.0.0.36+2

- Default to showing podspec lint warnings.
- Default to showing podspec lint warnings

## v.0.0.36+1

Expand Down
25 changes: 15 additions & 10 deletions script/tool/lib/src/version_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,11 @@ enum _CurrentVersionState {
/// The version is unchanged.
unchanged,

/// The version has changed, and the transition is valid.
validChange,
/// The version has increased, and the transition is valid.
validIncrease,

/// The version has decrease, and the transition is a valid revert.
validRevert,

/// The version has changed, and the transition is invalid.
invalidChange,
Expand Down Expand Up @@ -218,7 +221,8 @@ class VersionCheckCommand extends PackageLoopingCommand {
case _CurrentVersionState.unchanged:
versionChanged = false;
break;
case _CurrentVersionState.validChange:
case _CurrentVersionState.validIncrease:
case _CurrentVersionState.validRevert:
versionChanged = true;
break;
case _CurrentVersionState.invalidChange:
Expand All @@ -232,7 +236,7 @@ class VersionCheckCommand extends PackageLoopingCommand {
}

if (!(await _validateChangelogVersion(package,
pubspec: pubspec, pubspecVersionChanged: versionChanged))) {
pubspec: pubspec, pubspecVersionState: versionState))) {
errors.add('CHANGELOG.md failed validation.');
}

Expand Down Expand Up @@ -322,7 +326,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
'${getBoolArg(_againstPubFlag) ? 'on pub server' : 'at git base'}.');
logWarning(
'${indentation}If this plugin is not new, something has gone wrong.');
return _CurrentVersionState.validChange; // Assume new, thus valid.
return _CurrentVersionState.validIncrease; // Assume new, thus valid.
}

if (previousVersion == currentVersion) {
Expand All @@ -340,7 +344,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
if (possibleVersionsFromNewVersion.containsKey(previousVersion)) {
logWarning('${indentation}New version is lower than previous version. '
'This is assumed to be a revert.');
return _CurrentVersionState.validChange;
return _CurrentVersionState.validRevert;
}
}

Expand All @@ -367,7 +371,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
return _CurrentVersionState.invalidChange;
}

return _CurrentVersionState.validChange;
return _CurrentVersionState.validIncrease;
}

/// Checks whether or not [package]'s CHANGELOG's versioning is correct,
Expand All @@ -378,7 +382,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
Future<bool> _validateChangelogVersion(
RepositoryPackage package, {
required Pubspec pubspec,
required bool pubspecVersionChanged,
required _CurrentVersionState pubspecVersionState,
}) async {
// This method isn't called unless `version` is non-null.
final Version fromPubspec = pubspec.version!;
Expand All @@ -405,8 +409,9 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
// changes that don't warrant publishing on their own.
final bool hasNextSection = versionString == 'NEXT';
if (hasNextSection) {
// NEXT should not be present in a commit that changes the version.
if (pubspecVersionChanged) {
// NEXT should not be present in a commit that increases the version.
if (pubspecVersionState == _CurrentVersionState.validIncrease ||
pubspecVersionState == _CurrentVersionState.invalidChange) {
printError(badNextErrorMessage);
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions script/tool/test/util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -319,14 +319,14 @@ String _pluginPlatformSection(
return entry;
}

typedef ErrorHandler = void Function(Error error);
typedef _ErrorHandler = void Function(Error error);

/// Run the command [runner] with the given [args] and return
/// what was printed.
/// A custom [errorHandler] can be used to handle the runner error as desired without throwing.
Future<List<String>> runCapturingPrint(
CommandRunner<void> runner, List<String> args,
{ErrorHandler? errorHandler}) async {
{_ErrorHandler? errorHandler}) async {
final List<String> prints = <String>[];
final ZoneSpecification spec = ZoneSpecification(
print: (_, __, ___, String message) {
Expand Down
31 changes: 29 additions & 2 deletions script/tool/test/version_check_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class MockProcessResult extends Mock implements io.ProcessResult {}

void main() {
const String indentation = ' ';
group('$VersionCheckCommand', () {
group('VersionCheckCommand', () {
late FileSystem fileSystem;
late MockPlatform mockPlatform;
late Directory packagesDir;
Expand Down Expand Up @@ -602,7 +602,7 @@ This is necessary because of X, Y, and Z
);
});

test('Fail if the version changes without replacing NEXT', () async {
test('fails if the version increases without replacing NEXT', () async {
final RepositoryPackage plugin =
createFakePlugin('plugin', packagesDir, version: '1.0.1');

Expand Down Expand Up @@ -632,6 +632,33 @@ This is necessary because of X, Y, and Z
);
});

test('allows NEXT for a revert', () async {
final RepositoryPackage plugin =
createFakePlugin('plugin', packagesDir, version: '1.0.0');

const String changelog = '''
## NEXT
* Some changes that should be listed as part of 1.0.1.
## 1.0.0
* Some other changes.
''';
createFakeCHANGELOG(plugin, changelog);
createFakeCHANGELOG(plugin, changelog);
processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
MockProcess(stdout: 'version: 1.0.1'),
];

final List<String> output = await runCapturingPrint(
runner, <String>['version-check', '--base-sha=main']);
expect(
output,
containsAllInOrder(<Matcher>[
contains('New version is lower than previous version. '
'This is assumed to be a revert.'),
]),
);
});

test(
'fails gracefully if the version headers are not found due to using the wrong style',
() async {
Expand Down

0 comments on commit 1a124b1

Please sign in to comment.