[tool] Add config options needed by core-packages#11784
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces min_dart as an alternative to min_flutter in the repository tool configuration, updating the validation logic to check Dart SDK constraints accordingly. It also makes the .ci.yaml file optional during validation when batched releases are not used. Review feedback suggests handling potential FormatException errors when parsing version strings and correcting a configuration mismatch in the newly added tests where isFlutter: true was incorrectly set for non-Flutter package test cases.
| }) { | ||
| final errors = <String>[]; | ||
| final File ciYamlFile = repoRoot.childFile('.ci.yaml'); | ||
| if (!ciYamlFile.existsSync()) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Because the next non-new line in this file unconditionally reads ciYamlFile, and that will explode if, as is currently the case in core-packages, there is no such file.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
yeah the same will apply if we use GA to run ci, but this helper method is specific for ci.yaml.
There was a problem hiding this comment.
oh I see the reasoning, so until we implement the GA restriction, batch release is not supported. yeah I think that makes sense too.
Adds some optional behaviors needed to use the tool in flutter/core-packages: