-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[tool] Add config options needed by core-packages #11784
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
dcee7c8
9b54907
79c2c00
8321c79
e5ce7bb
485d0cf
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 |
|---|---|---|
|
|
@@ -428,6 +428,17 @@ class RepoInfoValidator { | |
| }) { | ||
| final errors = <String>[]; | ||
| final File ciYamlFile = repoRoot.childFile('.ci.yaml'); | ||
| if (!ciYamlFile.existsSync()) { | ||
|
Contributor
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. why do we bar batch release based on .ci.yaml? IIRC, batch release infra are mostly based on GA and didn't rely on ci.yaml. Or is this a indirect way to disable the batch release for core-package?
Collaborator
Author
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. Because the next non-new line in this file unconditionally reads Rather than speculatively implement an alternate codepath that does the same validation for a non-LUCI-based configuration that we've never built and may never need, I'm just describing the actual state of the tooling.
Contributor
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. oh ok, I think we can just return without the print error then. The reason behind this check for batch release is the we have to add a enable_branch in ci.yaml to make sure the luci ci runs. If it doesn't exist, then we don't need to worry about having the enable_branch check.
Collaborator
Author
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. Why wouldn't the same basic requirement apply to a repo using GA to run CI, given that GA also has branch restrictions in configurations?
Contributor
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. yeah the same will apply if we use GA to run ci, but this helper method is specific for ci.yaml.
Contributor
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. oh I see the reasoning, so until we implement the GA restriction, batch release is not supported. yeah I think that makes sense too. |
||
| if (isBatchRelease) { | ||
| printError( | ||
| '${_indentation}Batched release is currently only supported for ' | ||
| 'repos using .ci.yaml', | ||
| ); | ||
| errors.add('Missing .ci.yaml'); | ||
| } | ||
| return errors; | ||
| } | ||
|
|
||
| final String content = ciYamlFile.readAsStringSync(); | ||
| final yaml = loadYaml(content) as YamlMap; | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.