Added dynamic feature documentation. #159
Conversation
jimmyca15
force-pushed
the
user/jimmyca/mReadme3
branch
from
8c88391 to
b09f85c
Compare
| ], | ||
| "properties": { | ||
| "Default": { | ||
| "type":"boolean" |
There was a problem hiding this comment.
I remember we require one of the variants to be default. Not sure how to enforce it.
There was a problem hiding this comment.
I did a search, it doesn't look like a constraint that can be enforced via JSON schema.
| @@ -0,0 +1,17 @@ | |||
| # Configuration Schemas | |||
There was a problem hiding this comment.
Is your plan to update https://github.com/Azure/AppConfiguration/tree/main/docs/FeatureManagement pointing to here?
There was a problem hiding this comment.
It wasn't. Do you see some value in that?
There was a problem hiding this comment.
Never mind. That's the schema for AppConfig, which needs a new schema for dynamic feature support of its own. No need to link here.
README.md
Outdated
| .AddFeatureVariantAssigner<TargetingFeatureVariantAssigner>(); | ||
| ``` | ||
|
|
||
| ### Variant Value Resolution |
There was a problem hiding this comment.
How about just "Variant Resolution"?
The word "value" in this document is confusing to me. We don't assign a "value" to a feature. A variant is simply a configuration of a feature. The configuration can be bound to a strongly typed object, but it doesn't change the fact that it's just the configuration of a feature, not the feature itself.
There was a problem hiding this comment.
I see. I'd like to discuss this one a bit to see if we can land in the best place with this doc section and the interface associated with it.
* Moved around sections * Removed enum usage. * Fixed mistyped interfaces. * Clarified variant resolution.
README.md
Outdated
| IFeatureManager featureManager; | ||
| … | ||
| if (await featureManager.IsEnabledAsync(nameof(MyFeatureFlags.FeatureU))) | ||
| if (await featureManager.IsEnabledAsync(MyFeatureFlags.FeatureU)) |
There was a problem hiding this comment.
The enum MyFeatureFlags is removed, so it should be just string "FeatureU".
There was a problem hiding this comment.
It's meant to be a static class like this. Do you think it's unintuitive?
There was a problem hiding this comment.
I see. Then we should have MyFeatureFlags defined somewhere in this doc. I would just go with the string directly. It's easier to understand (and less distracting).
There was a problem hiding this comment.
The only concern I had is that just using IsEnabledAsync(FeatureV) kind of looks like we meant to use a string but forgot quotes.
There was a problem hiding this comment.
I meant to use the string directly, so It should be IsEnabledAsync("FeatureV").
There was a problem hiding this comment.
Ah yes, you did put a literal. Okay, I'm fine with that.
README.md
Outdated
| * [Feature Flag Declaration](./README.md#Feature-Flag-Declaration) | ||
| * [Feature Filters](./README.md#Feature-Filters) | ||
| * [ASP.NET Core Integration](./README.md#ASPNET-Core-Integration) | ||
| * [Targeting](./README.md#Targeting) |
There was a problem hiding this comment.
I don't see a section that is named "Targeting". Do you mean to point to the "Microsoft.Targeting" section?
BTW, to differentiate from the "Targeting assigner" below, can we name this "Targeting filter"?
There was a problem hiding this comment.
That's this section. The section explains the concept of targeting. Since it's a bit of a generic explanation I moved it to after feature flags and dynamic features. Even though it does reference feature filters, it uses more of a tool in the explanation of targeting. So now, the targeting feature filter and the targeting feature variant assigner appear symmetric in the index, then the generic targeting explanation appears at a top level.
* Add support for feature variants. * Renaming and assorted fixes. * Remove unnecessary field. * * Added additional argument validation. * Added additional dynamic feature configuration validation. * Fixed bug in audience accumulation for variant targeting * Added test for variant targeting audience accumulation. * Since filter aliases are case insensitive, update filter cache to reflect this. * * Fixed issue with filter + assigner suffix references. * Clarified error types. * Updated/added tests * Share suffix matching logic. Corrected variable name. * Removed unnecessary helper methods. Updated error codes for consistency. * Clarify filter selection logic in feature evaluation and add checks for impossible contextual evaluations. * Updated argument validation messages. Updated exception used for null child property. * Update to dynamic features. (#157) * Update to dynamic features. * Update types to respect dynamic feature name. E.g. IFeatureManager -> IDynamicFeatureManager. * Updated property/method summaries to distinguish dynamic features and feature flags. * Added IDynamicFeatureManagerSnapshot * Separate feature flags and dynamic features in configuration schema * Backward compatability support for old configuration schema. * Breaking: Update custom feature definition provider to return feature flags and dynamic features. * Added DynamicFeatureDefinition class * renamed FeatureDefinition to FeatureFlagDefinition. * Breaking: IFeatureManager.GetFeatureNamesAsync renamed to IFeatureManager.GetFeatureFlagNamesAsync * Corrected namespace for built-in feature variant assigners. * Microsoft.FeatureManagement -> Microsoft.FeatureManagement.Assigners * Added test for custom feature definition provider. * Added test for backcompat support for v1 configuration schema. * * Separated feature flag implementation from dynamic feature impelementation in FeatureManager, ConfigurationFeatureDefinitionProvider, and FeatureManagerSnapshot.. * Separated IConfigurationDefinitionProvider into IFeatureFlagDefinitionProvider and IDynamicFeatureDefinitionProvider * Added missing default cancellation tokens. * Use shared helper for filter/assigner reference matching. * Add copyright header. * Added dynamic feature documentation. (#159) * Added dynamic feature documentation. * Readme fixes. * * Added index. * Moved around sections * Removed enum usage. * Fixed mistyped interfaces. * Clarified variant resolution. * Fix links with '.' character. * Updates. * Updated index. * Default -> Name * Remove `MyFeatureFlags` references. * Updated missed "feature name" occurrences. * feature definition -> feature flag * SuggestedReadmeChanges * Updated comments from readme * Fixed targeting assignment precedence. * Remove unused variable. Update method signature. Added missing declaration in test. * Add null/empty check for feature evaluation. * Added .NET 6 as a target framework and removed .NET 5 * Update Azure App Configuration provider reference in example project to pull features using v2 schema. * Build with .NET 6 (#186) * Build with .NET 6 * Update build step name. * Add link to v2 readme. (#184) * Throw exception if feature management schemas are mixed to avoid unintentionally hiding feature flags. (#187) Co-authored-by: mrm9084 <[email protected]>
3 participants
This PR adds