-
Notifications
You must be signed in to change notification settings - Fork 102
Add a server flag onlyUseTipOfTreeTargetsExistenceToFilterTargets.
#4731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,8 @@ final class CiYamlSet { | |
| required this.slug, | ||
| required this.branch, | ||
| required Map<CiType, pb.SchedulerConfig> yamls, | ||
| bool onlyUseTipOfTreeTargetsExistenceToFilterTargets = | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we make this required without reaching out to Config.kOnlyUseTipOfTreeTargetsExistenceToFilterTargets? |
||
| Config.kOnlyUseTipOfTreeTargetsExistenceToFilterTargets, | ||
| CiYamlSet? totConfig, | ||
| bool validate = false, | ||
| }) { | ||
|
|
@@ -50,6 +52,8 @@ final class CiYamlSet { | |
| type: type, | ||
| validate: validate, | ||
| totConfig: totConfig?.configs[type], | ||
| onlyUseTipOfTreeTargetsExistenceToFilterTargets: | ||
| onlyUseTipOfTreeTargetsExistenceToFilterTargets, | ||
| ); | ||
| } | ||
| } | ||
|
|
@@ -117,7 +121,8 @@ class CiYaml { | |
| required this.branch, | ||
| required this.config, | ||
| required this.type, | ||
| this.onlyUseTipOfTreeTargetsExistenceToFilterTargets = false, | ||
| this.onlyUseTipOfTreeTargetsExistenceToFilterTargets = | ||
| Config.kOnlyUseTipOfTreeTargetsExistenceToFilterTargets, | ||
| CiYaml? totConfig, | ||
| bool validate = false, | ||
| }) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,9 @@ | |
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| /// @docImport '../model/ci_yaml/ci_yaml.dart'; | ||
| library; | ||
|
|
||
| import 'dart:convert'; | ||
| import 'dart:math' show Random; | ||
| import 'dart:typed_data'; | ||
|
|
@@ -31,12 +34,18 @@ const String kDefaultBranchName = 'master'; | |
|
|
||
| interface class Config { | ||
| /// Creates and returns a [Config] instance. | ||
| Config(this._cache, this._secrets, {required DynamicConfig dynamicConfig}) | ||
| : _dynamicConfig = dynamicConfig; | ||
| Config( | ||
| this._cache, // | ||
| this._secrets, { | ||
| required DynamicConfig dynamicConfig, | ||
| }) : _dynamicConfig = dynamicConfig; | ||
|
|
||
| /// Access dynamically configured flags. | ||
| DynamicConfig get flags => _dynamicConfig; | ||
|
|
||
| /// See [CiYaml.onlyUseTipOfTreeTargetsExistenceToFilterTargets]. | ||
| static const bool kOnlyUseTipOfTreeTargetsExistenceToFilterTargets = false; | ||
|
Comment on lines
+46
to
+47
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than exposing the default and maybe hiding places that do not pass this flag (error prone); make people refer to |
||
|
|
||
| /// When present on a pull request, instructs Cocoon to submit it | ||
| /// automatically as soon as all the required checks pass. | ||
| /// | ||
|
|
@@ -514,9 +523,19 @@ final class DynamicConfig { | |
|
|
||
| final ContentAwareHashingJson contentAwareHashing; | ||
|
|
||
| /// Whether to _only_ use the existence of a target at tip-of-tree to filter | ||
| /// out targets on other branches. | ||
| /// | ||
| /// See [CiYaml.onlyUseTipOfTreeTargetsExistenceToFilterTargets]. | ||
| @JsonKey( | ||
| defaultValue: Config.kOnlyUseTipOfTreeTargetsExistenceToFilterTargets, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. default this to |
||
| ) | ||
| final bool onlyUseTipOfTreeTargetsExistenceToFilterTargets; | ||
|
Comment on lines
525
to
+533
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see us having some other flags for ci_yaml; can we create a CiYamlFlags like ContentAwareHashingJson instead of expanding this? |
||
|
|
||
| DynamicConfig({ | ||
| required this.backfillerCommitLimit, | ||
| required this.contentAwareHashing, | ||
| required this.onlyUseTipOfTreeTargetsExistenceToFilterTargets, | ||
| }); | ||
|
|
||
| /// Connect the generated [_$DynamicConfigFromJson] function to the `fromJson` | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do this in its own PR so it can be reverted cleanly if needed.