Skip to content

Update the recommended way to reference feature flag name in README #275

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.

Sign up for GitHub

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

Merged
zhiyuanliang-ms merged 5 commits into from
Oct 13, 2023
Merged

Update the recommended way to reference feature flag name in README #275

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fdd29a4
update the recommended way to reference feature flag name
zhiyuanliang-ms Oct 9, 2023
c3cc21a
remove nameof
zhiyuanliang-ms Oct 10, 2023
d2c79ea
remove recommended referencing way
zhiyuanliang-ms Oct 11, 2023
8905db3
using string literals directly
zhiyuanliang-ms Oct 12, 2023
68ea7f0
Merge branch 'main' into zhiyuanliang/readme-feature-name-referencing
zhiyuanliang-ms Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

@jimmyca15 jimmyca15 Oct 9, 2023

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.

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.

@zhiyuanliang-ms zhiyuanliang-ms Oct 10, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.


``` C#
// Define feature flags in an enum
public enum MyFeatureFlags
// Define feature flags using const strings
public static class MyFeatureFlags
{
FeatureT,
FeatureU,
FeatureV
public const string FeatureT = "FeatureT";
public const string FeatureU = "FeatureU";
public const string FeatureV = "FeatureV";
}
```

Expand Down