Skip to content

Refactor and split up DynamicConfig*, prepare for more flag groups.#4732

Merged
auto-submit[bot] merged 2 commits into
flutter:mainfrom
matanlurey:dynamic-config-refactor
May 29, 2025
Merged

Refactor and split up DynamicConfig*, prepare for more flag groups.#4732
auto-submit[bot] merged 2 commits into
flutter:mainfrom
matanlurey:dynamic-config-refactor

Conversation

@matanlurey
Copy link
Copy Markdown
Contributor

Doing as pre-work before requested changes on #4731.

I ran into a number of cases where changes were needed, and figured a separate PR was cleaner:

  • Moved all of the bits out of config.dart and into flags/_*.dart files to reduce file size

  • We have command-line tools in app_dart/bin that are not a server (i.e. verify_test_ownership.dart) that create CiYaml objects, and also need CiYamlFlags. Instead of hacking something together, I added DynamicConfig.fromRemoteLatest and DynamicConfig.fromLocalRepo to use, respectively, in these command-line tools (which are used in CI production, not unused tools).

  • I made all of the objects const-able, and made it easy to create defaults, even if they are not const themselves, by using the factory constructor pattern. That also gets rid of the kludge we had with contentAwareHashing default values.

@matanlurey matanlurey requested a review from jtmcdole May 28, 2025 23:57
const String kDefaultBranchName = 'master';

interface class Config {
interface class Config extends DynamicallyUpdatedConfig {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

Comment thread app_dart/lib/src/service/flags/dynamic_config.dart Outdated
return DynamicConfigUpdater().fetchDynamicConfig();
}

/// Returns the latest copy of [DynamicConfig] fetched from the repository.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fromLocalFilesystem?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Comment thread app_dart/lib/src/service/flags/dynamic_config.dart Outdated
@matanlurey matanlurey requested a review from jtmcdole May 29, 2025 03:26
Copy link
Copy Markdown
Member

@jtmcdole jtmcdole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm_clap.mp4

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App. label May 29, 2025
@auto-submit auto-submit Bot merged commit 51381bc into flutter:main May 29, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants