-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[App Config] Add display_name to FeatureFlag #15901
Conversation
@@ -62,10 +66,12 @@ export const FeatureFlagHelper = { | |||
} | |||
const jsonFeatureFlagValue: JsonFeatureFlagValue = { | |||
id: featureFlag.value.id ?? key.replace(featureFlagPrefix, ""), | |||
...featureFlag.value, |
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.
Were enabled
and description
the only fields we would miss be removing this line?
I agree it makes sense to remove the line (or move it above the id
assignment), just want to ensure we aren't forgetting to set a field that was previously being set.
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.
@@ -91,6 +91,7 @@ export interface FeatureFlagValue { | |||
}[]; | |||
}; | |||
description?: string; | |||
displayName?: string; |
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.
So this field will eventually be used in the portal? (And presumably someone writing their own app config viewer could use it as well)
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.
yes, yess!
Hello @HarshaNalluru! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Fixes #15891