Skip to content

Commit d1910ea

Browse files
[tool] Unify most validation commands (#11641)
Replaces all of the following repo tooling commands with a new unified `validate` command: - `dependabot-check` - `gradle-check` - `pubspec-check` - `readme-check` - `repo-package-info-check` - `version-check` It also makes some simplifications in using these: - Temporarily disabling the code excerpt check is now done via package-level ci_config.yaml files. This means that we don't have to exempt a package from the entire command just for this, and also means that fixing a package and removing its exemption will no longer trigger whole-repo testing as it used to. - Setting the min Flutter version allowed for the repo is now a repo-level config in a new root level `tool_config/` directory. This avoids the need to pass a flag every time for something that isn't actually intended to be variable. - The allowed dependencies are now also in hard-coded locations in the `tool_config/` directory, instead of having to pass them in all the time, to make the commands easier to call. As with min SDK version, this isn't something callers actually need to be able to change on the fly to the flags add more complexity (in exchange for making it extremely clear at the CI call site what was happening, but I no longer think that tradeoff is the right one based on experience using the tool). To avoid huge amounts of test file churn, and massively complicating test setup, there is a test-only mechanism to select individual validators to run in tests, rather than having to test the entire command at once. Part of flutter/flutter#173413
1 parent d060692 commit d1910ea

35 files changed

Lines changed: 819 additions & 861 deletions
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ set -e
1010
# missing version/CHANGELOG detection since PR-level overrides aren't available
1111
# in post-submit.
1212
if [[ $LUCI_PR == "" ]]; then
13-
.ci/scripts/tool_runner.sh version-check --ignore-platform-interface-breaks
13+
.ci/scripts/tool_runner.sh validate --ignore-platform-interface-breaks
1414
else
15-
.ci/scripts/tool_runner.sh version-check --check-for-missing-changes --pr-labels="$PR_OVERRIDE_LABELS"
15+
.ci/scripts/tool_runner.sh validate --check-for-missing-changes --pr-labels="$PR_OVERRIDE_LABELS"
1616
fi

.ci/targets/repo_checks.yaml

Lines changed: 2 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -26,46 +26,12 @@ tasks:
2626
script: .ci/scripts/tool_runner.sh
2727
args: ["license-check"]
2828
always: true
29-
# The major and minor version here should match the lowest version analyzed
30-
# in legacy version analysis (.ci.yaml analyze_legacy).
31-
- name: pubspec validation
32-
script: .ci/scripts/tool_runner.sh
33-
args:
34-
- "pubspec-check"
35-
- "--min-min-flutter-version=3.35.0"
36-
- "--allow-dependencies=script/configs/allowed_unpinned_deps.yaml"
37-
- "--allow-pinned-dependencies=script/configs/allowed_pinned_deps.yaml"
38-
always: true
39-
- name: README validation
40-
script: .ci/scripts/tool_runner.sh
41-
args: ["readme-check"]
42-
always: true
43-
# Re-run with --require-excerpts, skipping packages that still need
44-
# to be converted. Once https://github.com/flutter/flutter/issues/102679
45-
# has been fixed, this can be removed --require-excerpts added to the
46-
# run above.
47-
- name: README snippet configuration validation
48-
script: .ci/scripts/tool_runner.sh
49-
args: ["readme-check", "--require-excerpts", "--exclude=script/configs/temp_exclude_excerpt.yaml"]
50-
always: true
5129
- name: README snippet validation
5230
script: .ci/scripts/tool_runner.sh
5331
args: ["update-excerpts", "--fail-on-change"]
5432
always: true
55-
- name: Gradle validation
56-
script: .ci/scripts/tool_runner.sh
57-
args: ["gradle-check"]
58-
always: true
59-
- name: Repository-level package metadata validation
60-
script: .ci/scripts/tool_runner.sh
61-
args: ["check-repo-package-info"]
62-
always: true
63-
- name: Dependabot coverage validation
64-
script: .ci/scripts/tool_runner.sh
65-
args: ["dependabot-check"]
66-
always: true
67-
- name: CHANGELOG and version validation
68-
script: .ci/scripts/check_version.sh
33+
- name: Repository standards validation
34+
script: .ci/scripts/validate.sh
6935
always: true
7036
- name: federated safety check
7137
script: .ci/scripts/check_federated_safety.sh

AGENTS.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,9 @@ dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart update-dependency
9090
The tool can also run native and integration tests, but these may require a more complete environment than is available.
9191
- **Validation**: Run these checks to ensure that changes follow team guidelines:
9292
```bash
93+
dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart validate --packages <changed_packages>
9394
dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart publish-check --packages <changed_packages>
94-
dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart readme-check --packages <changed_packages>
95-
dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart version-check --packages <changed_packages>
9695
dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart license-check
97-
dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart repo-package-info-check
9896
```
9997

10098
### Specialized Workflows

packages/espresso/ci_config.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# TODO(stuartmorgan): Remove this; see https://github.com/flutter/flutter/issues/102679
2+
exempt_from_excerpts: true
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# TODO(stuartmorgan): Remove this; see https://github.com/flutter/flutter/issues/102679
2+
exempt_from_excerpts: true
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# TODO(stuartmorgan): Remove this; see https://github.com/flutter/flutter/issues/102679
2+
exempt_from_excerpts: true
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# TODO(stuartmorgan): Remove this; see https://github.com/flutter/flutter/issues/102679
2+
exempt_from_excerpts: true

script/configs/temp_exclude_excerpt.yaml

Lines changed: 0 additions & 12 deletions
This file was deleted.

script/tool/README.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ dart run script/tool/bin/flutter_plugin_tools.dart format --packages package_nam
4949

5050
The `flutter/packages` repository uses clang version `15.0.0` . Newer versions of clang may format code differently.
5151

52-
### Run the Static Analysis
52+
### Run Static Analysis
5353

5454
To analyze only Dart code:
5555

@@ -68,6 +68,18 @@ dart run script/tool/bin/flutter_plugin_tools.dart analyze --ios --macos --packa
6868

6969
Dart analysis can be excluded with `--no-dart`.
7070

71+
### Run General Validation
72+
73+
To check that changes follow team standards and best practices, run:
74+
75+
```sh
76+
dart run script/tool/bin/flutter_plugin_tools.dart validate --check-for-missing-changes --packages package_name
77+
```
78+
79+
If you are making changes that fall under a CHANGELOG and/or version change
80+
exemption you can omit the `--check-for-missing-changes` flag to skip those
81+
checks.
82+
7183
### Run Dart Unit Tests
7284

7385
```sh

script/tool/lib/src/common/ci_config.dart

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import 'package:yaml/yaml.dart';
77
/// A class representing the parsed content of a `ci_config.yaml` file.
88
class CIConfig {
99
/// Creates a [CIConfig] from a parsed YAML map.
10-
CIConfig._(this.isBatchRelease);
10+
CIConfig._({required this.isBatchRelease, required this.requiresExcerpts});
1111

1212
/// Parses a [CIConfig] from a YAML string.
1313
///
@@ -26,18 +26,29 @@ class CIConfig {
2626
isBatchRelease = release['batch'] == true;
2727
}
2828

29-
return CIConfig._(isBatchRelease);
29+
// Any package that hasn't been explicitly exempted is assumed to require
30+
// excerpts.
31+
final requiresExcerpts = loaded['exempt_from_excerpts'] != true;
32+
33+
return CIConfig._(
34+
isBatchRelease: isBatchRelease,
35+
requiresExcerpts: requiresExcerpts,
36+
);
3037
}
3138

3239
static const Map<String, Object?> _validCIConfigSyntax = <String, Object?>{
3340
'release': <String, Object?>{
3441
'batch': <bool>{true, false},
3542
},
43+
'exempt_from_excerpts': <bool>{true, false},
3644
};
3745

3846
/// Returns true if the package is configured for batch release.
3947
final bool isBatchRelease;
4048

49+
/// Returns true if the package is configured to require excerpts.
50+
final bool requiresExcerpts;
51+
4152
static void _checkCIConfigEntries(
4253
YamlMap config, {
4354
required Map<String, Object?> syntax,

0 commit comments

Comments
 (0)