-
Notifications
You must be signed in to change notification settings - Fork 37
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
Improve summaries for methods in AzureAppConfigurationOptions #467
Conversation
@@ -118,6 +118,7 @@ internal IEnumerable<IKeyValueAdapter> Adapters | |||
/// <summary> | |||
/// Specify what key-values to include in the configuration provider. | |||
/// <see cref="Select"/> can be called multiple times to include multiple sets of key-values. | |||
/// If neither <see cref="Select"/> nor <see cref="SelectSnapshot"/> is ever called, all key-values with null label are included in the configuration provider. |
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.
The intent seems good, but it's a bit odd that it's on the Select method. I guess I would expect it on AppConfigurationOptions summary.
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.
Yea I agree honestly, I wasn't sure if adding it to the options summary seemed random/too much information there. Probably a bit better than this
src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com>
Add mention of default query in the summary for
AzureAppConfigurationOptions
. Fix tags in summary forAzureAppConfigurationOptions.UseFeatureFlags
.