Update the recommended way to reference feature flag name in README #275
Conversation
README.md
Outdated
| @@ -144,12 +144,12 @@ In the above example, `FeatureW` specifies a `RequirementType` of `All`, meaning | |||
| To make it easier to reference these feature flags in code, we recommend to define feature flag variables like below. | |||
There was a problem hiding this comment.
we recommend to define feature flag variables like below
I'm thinking, is it worth a recommendation? This is fairly basic definition of strings. I would imagine just removing this section would be fine.
The only issue I can think with that is that we reference MyFeatureFlags.FeatureX and others below. Perhaps though, it's clear enough that those references should be the names of the flags.
There was a problem hiding this comment.
If we want to remove the recommendation, maybe we can just use string "FeatureX" instead of the reference.
If I am a user, I can understand that this is just the name of the feature flag which is a string variable.
But there is a customer who raised an issue for this. He expected us to update the recommendation.
|
References of |
README.md
Outdated
| public const string FeatureV = "FeatureV"; | ||
| } | ||
| ``` | ||
| In the following code examples, MyFeatureFlags.FeatureX which is a const string property of the MyFeatureFlags class, is used for referencing the name of feature flag "FeatureX". |
There was a problem hiding this comment.
What do you think about just removing MyFeatureFlags altogether and using string literals?
There was a problem hiding this comment.
Sure. That's my original thought #275 (comment)
zhiyuanliang-ms
deleted the
zhiyuanliang/readme-feature-name-referencing
branch
zhiyuanliang-ms
restored the
zhiyuanliang/readme-feature-name-referencing
branch
zhiyuanliang-ms
deleted the
zhiyuanliang/readme-feature-name-referencing
branch
Issue #269
Change the recommended approach to reference the feature flag names from enum to const strings.