Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new console sample that demonstrates using FeatureFilterConfiguration.ParametersObject to supply strongly-typed filter settings directly from a custom IFeatureDefinitionProvider, avoiding IConfiguration-based parameter binding.
Changes:
- Introduces
ParameterObjectConsoleAppexample project with a top-levelProgram.csthat evaluates a targeting-based feature for multiple users. - Adds an
InMemoryFeatureDefinitionProviderthat returns aFeatureDefinitionwhose filter settings are provided viaParametersObject. - Documents the concept and usage in a new sample
README.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| examples/ParameterObjectConsoleApp/README.md | Documents the new ParametersObject-based sample and how to run it. |
| examples/ParameterObjectConsoleApp/Program.cs | Implements the sample’s runtime evaluation loop using ContextualTargetingFilter. |
| examples/ParameterObjectConsoleApp/ParameterObjectConsoleApp.csproj | Defines the new .NET 8 console app project and references the main library. |
| examples/ParameterObjectConsoleApp/InMemoryFeatureDefinitionProvider.cs | Provides in-memory feature definitions using ParametersObject for targeting settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| await Task.CompletedTask; |
There was a problem hiding this comment.
GetAllFeatureDefinitionsAsync is implemented as an async iterator but only awaits Task.CompletedTask at the end, which is redundant and adds noise. Consider removing the trailing await (and/or dropping async if you switch to a non-async implementation) so the method is a straightforward async iterator.
| await Task.CompletedTask; |
| // | ||
| using Microsoft.FeatureManagement; | ||
| using Microsoft.FeatureManagement.FeatureFilters; | ||
| namespace ParameterObjectConsoleApp |
There was a problem hiding this comment.
The folder/project/namespace name uses ParameterObject... (singular) while the API being demonstrated is ParametersObject (plural). Consider aligning the example name with the public API name to reduce confusion when users search for it.
| namespace ParameterObjectConsoleApp | |
| namespace ParametersObjectConsoleApp |
| ## Running | ||
|
|
||
| ``` | ||
| dotnet run |
There was a problem hiding this comment.
The running instructions are ambiguous from the repo root (plain dotnet run will likely execute a different project or error). Consider updating this section to either include a cd examples/ParameterObjectConsoleApp step or use dotnet run --project examples/ParameterObjectConsoleApp/ParameterObjectConsoleApp.csproj.
| dotnet run | |
| dotnet run --project examples/ParameterObjectConsoleApp/ParameterObjectConsoleApp.csproj |
| <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.1" /> | ||
| <PackageReference Include="System.Text.Json" Version="8.0.5" /> | ||
| <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.1" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> |
There was a problem hiding this comment.
The project references configuration/JSON/DI packages, but this example doesn't use IConfiguration, JSON serialization, or DI. Consider removing these PackageReferences to keep the sample minimal (the FeatureManagement project reference already brings in what it needs transitively).
| <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.1" /> | |
| <PackageReference Include="System.Text.Json" Version="8.0.5" /> | |
| <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.1" /> | |
| </ItemGroup> | |
| <ItemGroup> |
No description provided.