Skip to content
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

migrate experimental-stop-grpc-service-on-defrag flag to feature gate. #18359

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

siyuanfoundation
Copy link
Contributor

@siyuanfoundation siyuanfoundation commented Jul 23, 2024

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

part of #18023, an example of basic wiring from an existing experimental feature flag to the new feature gate.

final step of of breaking up #18234 into smaller PRs.

@siyuanfoundation
Copy link
Contributor Author

/cc @serathius @ahrtr @fuweid

// SetFeatureGatesFromExperimentalFlags sets the feature gate values if the feature gate is not explicitly set
// while their corresponding experimental flags are explicitly set.
// TODO: remove after all experimental flags are deprecated.
func SetFeatureGatesFromExperimentalFlags(fg featuregate.FeatureGate, getExperimentalFlagVal func(string) (bool, bool), featureGatesFlagName, featureGatesVal string) error {
Copy link
Member

@serathius serathius Jul 24, 2024

Choose a reason for hiding this comment

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

Mark as deprecated? so it's clear it's not assumed to be the recommended option for new flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Marking ExperimentalFlagToFeatureMap as deprecated, because that's where the actual experimental features are added.

server/etcdmain/help.go Outdated Show resolved Hide resolved
@siyuanfoundation siyuanfoundation force-pushed the fg-2 branch 2 times, most recently from 1f09b93 to 95a1c65 Compare July 24, 2024 17:54
@siyuanfoundation
Copy link
Contributor Author

/retest

server/etcdmain/config.go Outdated Show resolved Hide resolved
server/etcdmain/config.go Outdated Show resolved Hide resolved
server/etcdmain/config.go Outdated Show resolved Hide resolved
@siyuanfoundation siyuanfoundation marked this pull request as draft July 25, 2024 16:28
@siyuanfoundation siyuanfoundation force-pushed the fg-2 branch 3 times, most recently from eef3b6f to ffb03a2 Compare July 26, 2024 19:51
@siyuanfoundation siyuanfoundation marked this pull request as ready for review July 26, 2024 19:52
@siyuanfoundation
Copy link
Contributor Author

/retest

pkg/flags/flag.go Outdated Show resolved Hide resolved
server/etcdmain/help.go Outdated Show resolved Hide resolved
server/embed/config.go Outdated Show resolved Hide resolved
@ahrtr
Copy link
Member

ahrtr commented Aug 1, 2024

cc @fuweid @ivanvc @jmhbnz @serathius

Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

server/embed/config_test.go Outdated Show resolved Hide resolved
@siyuanfoundation
Copy link
Contributor Author

/retest

@siyuanfoundation siyuanfoundation requested a review from ahrtr August 6, 2024 17:56
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks

@ahrtr
Copy link
Member

ahrtr commented Aug 7, 2024

cc @jmhbnz @serathius

@serathius serathius merged commit e34a101 into etcd-io:main Aug 7, 2024
44 checks passed
@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, fuweid, serathius, siyuanfoundation

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants