Documenting Options change for 2.0 #4797

guardrex · 2017-11-15T05:13:01Z

Configuration (internal review topic)
Options (internal review topic)

Notes

The sample had a few issues:
- It was broken OOB for IOptionsSnapshot.
- It relied on commenting and un-commenting compiler directives to see the various Options configurations.
- It was a crappy 'ole MVC thing. Yuck! 😄
Sample update
- Everything except ConfigureAll and IPostConfigureOptions are shown in the sample, and those two are described with examples in the topic. Everything works in the new sample.
- This sample cleanly demonstrates every feature with careful use of snippets and highlighting so that the dev only needs to focus two files (Startup.cs and Index.cshtml.cs) and run the app to see everything. There's no flipping compiler directives in two places (Startup and HomeController files) as the original sample requires in order to see all of the Options features.
- This is a ✨ shiny new Razor Pages sample ✨. ooooo ahhhhh!
The layout of the topic for Options has been overhauled. We have about eight content feature areas to cover, so I make one 2nd level "Options" heading, then I get our ~8 sub-content areas in there as 3rd level content. Most of them tie directly into the sample, and I use "Example #x" to keep the reader connected to the sample. Nobody gets lost.

Finally, I left old links and old [!NOTE]s in there b/c I do need to convince you that this is a much better way to go. I recommend you download the sample and check it out. I put it over here for easy access 👉 https://github.com/guardrex/UsingOptionsRazorPagesSampleFormat

Before I made that RP app, I just updated the existing one. I don't care for this, but here it is in case you want to see the difference 👉 https://github.com/guardrex/UsingOptionsOriginalSampleFormat

... and keep in mind that if we go with the original approach that all of those horrific [!NOTES] (commented-out right now) in the topic need to appear in the topic. Devs must be told how to enable/disable those compiler directives for each example. The notes explain what to do, but imo these are horrible instructions to follow.

guardrex · 2017-11-15T20:27:51Z

@Rick-Anderson @HaoK Ready for a look-see now. 👁️👁️

HaoK · 2017-11-15T20:34:00Z

Is it worth considering splitting Options into its own top level topic/section since Options is actually the what most of our frameworks rely on, where Configuration is just one way for options to be set.

HaoK · 2017-11-15T20:34:37Z

including @ajcvickers for review as well

HaoK · 2017-11-15T20:36:51Z

I think the code snippets are a bit verbose looking, since they are currently showing more than the current flow, i.e. for Basic options configuration (example 1) there is a scary amount of code in the snippets :)

The highlighted lines are fine, but all the surrounding code I think should be omitted

guardrex · 2017-11-15T20:42:18Z

Is it worth considering splitting Options into its own top level topic/section since Options is actually the what most of our frameworks rely on, where Configuration is just one way for options to be set.

We can do that. I didn't just because it isn't currently setup that way and there was no direction to add the content as a separate topic. @Rick-Anderson, how do u want to play it?

I think the code snippets are a bit verbose looking, since they are currently showing more than the current flow, i.e. for Basic options configuration (example 1) there is a scary amount of code in the snippets :)

The highlighted lines are fine, but all the surrounding code I think should be omitted

Yes, that's good. Let me try a version on the next commit that puts those into individual snippets so that those areas aren't overwhelmed with unnecessary statements.

... but before I do that ... I'll wait to hear back from @Rick-Anderson if he wants this entire Options section cut out into a separate topic ... then if "yes," I'll do both changes in one shot.

HaoK · 2017-11-15T20:45:20Z

So before drilling too far into the specifics of any section, I think this topic is too long right now.

My suggestion would be to have two separate topics for Options and Configuration, and then in Configuration you cover the ConfigurationBinder the options Configure overloads that take configuration sections which is how you bind options to configuration.

There's a bit of conflation between the two concepts right now when there's only a single topic. Like "configuration" is used to mean the file, and sometimes to mean "configuration of an option". Which is maybe why its a single topic today, but I think its better for us to explain the pieces individually and how they play with each other.

guardrex · 2017-11-15T21:04:02Z

I'm on the fence about splitting them, but I think you're right that if I just even more strategically use snippets to cut that extra code from the examples that it will come down in length quite nicely.

I'm just a little worried tho about splitting given that we probably don't want folks to accidentally miss the Options topic or think its less important and go on to adopting the bad practice of just loading IConfiguration everywhere and never using the options pattern. Since I know that might be a problem with splitting, I'll be careful if it comes to that to push the options pattern in the Configuration topic and link it well.

Rick-Anderson · 2017-11-15T21:16:24Z

Can we split them up and then refer to each other so they don't miss anything?

HaoK · 2017-11-15T21:19:14Z

Or you can just have a single Options & Configuration top level TOC item, and have Options and Configuration as siblings?

guardrex · 2017-11-15T21:36:46Z

Ok ... I'll get these done on the next commit and ping back.

guardrex · 2017-11-15T22:57:36Z

@Rick-Anderson

Option A: Do you want a main landing page "Configuration & options" as index with the two topics, Configuration and Options, linked from the index?

aspnetcore/fundamentals/configuration/index (Configuration & options landing page)
aspnetcore/fundamentals/configuration/configuration (the Configuration topic)
aspnetcore/fundamentals/configuration/options (the Options topic)

## [Configuration & options](xref:fundamentals/configuration/index)
### [Configuration](xref:fundamentals/configuration/configuration)
### [Options](xref:fundamentals/configuration/options)

Option B: ... or do you want the index to be the Configuration topic? I'll still load them side-by-side in the TOC, but there won't be a landing page ... it will shoot them right to the Configuration topic:

aspnetcore/fundamentals/configuration/index (the Configuration topic)
aspnetcore/fundamentals/configuration/options (the Options topic)

## [Configuration & options](xref:fundamentals/configuration/index)
### [Configuration](xref:fundamentals/configuration/index)
### [Options](xref:fundamentals/configuration/options)

(I'm guessing B here. 50-50 shot ... go ahead Rex ... roll the dice and take your chances!)

guardrex · 2017-11-16T00:36:59Z

@HaoK @Rick-Anderson See how that looks now.

👇 👇 👇

Configuration (internal review topic)
Options (internal review topic)

Update Updates Updates

Fix link Update TOC entry Fix redo Updates

guardrex · 2017-11-20T17:33:22Z

@HaoK 🍖 Happy Thanksgiving! 🦃

I just resolved the conflicts here. I'm OOF on vacation this week. If you provide feedback this week, I'll get it squared away after I get back on Monday, 11/27. Catch ya on the flip-side.

Simple is banned.

HaoK · 2017-11-20T23:39:49Z

I think you should move the second half of the "In-memory provider and binding to a POCO class" section to Options, basically everything after: "The following sample shows how to bind to MyWindow and use the options pattern with a ASP.NET Core MVC app:"

This would let you remove the overview of options as well in the configuration section, and just point that (and the earlier reference: "The options pattern is described later in this article.") to the Options topic

guardrex · 2017-11-21T01:12:58Z

I think you should move the second half of the "In-memory provider and binding to a POCO class" section to Options

That approach is already described in the (new) Options topic, so it can just be eliminated. I updated the references to Options: I point that one spot "described later in this article" to the Options topic. I remove the Options section and put a link to the Options topic in the intro.

In the "ObjectGraph" example, I update the use of "AppOptions" to "AppSettings" and "options" to "settings" to avoid confusion over the use of the word "options."

HaoK · 2017-11-21T22:24:24Z

Sounds good,

Also even tho you already do somewhat mention the behavior changes to IOptionsSnapshot in 2.0 in the Reloading config section, maybe its worth a sentence or two calling out what's changed explicitly since it might be easy to miss unless they can diff tabs better than I can :) (see aspnet/Options#217, I can also clarify exactly what changed if needed)

HaoK · 2017-11-21T22:27:57Z

"Configuring options after configuration with IPostConfigureOptions" sounds a bit...awkward.

Maybe "Using IPostConfigureOptions to normalize/validate/set defaults, and then you can add a small example like services.PostConfigure<SomeOption>(o => o.Message = o.Message ?? "Oops, I did it again!")

guardrex · 2017-11-21T22:32:50Z

@HaoK Sure ... I'll get on these next Monday. If I do one more thing on this repo while on "vacation" (whatever that is 😀), @Rick-Anderson will send his henchmen over here to teach me a lesson. 👊

HaoK · 2017-11-21T22:39:35Z

For the "Named options support with IConfigureNamedOptions" section, here's some relevant context that should be included: (feel free to edit as needed, but the rough idea is)

In 2.0, all options are now named instances. Any existing IConfigureOptions is simply treated as targeting the Options.DefaultName (which is string.Empty) instance. IConfigureNamedOptions also implement IConfigureOptions, and the default OptionsFactory has logic to use each appropriately. The other special name is the null name, which is used to signal targeting all named instances instead of a specific instance ([Post]ConfigureAll use this convention).

HaoK · 2017-11-21T22:40:29Z

You should suppress your github notifications while on vacation @guardrex :)

guardrex · 2017-11-21T23:57:18Z

some relevant context

Ah! ... but relevant to whom??!! 😄

I was aware of that context, but I chose not to include it. I'm concerned that those implementation details become just one more thing piled on to overwhelm the poor new dev or dev coming in cold from netfx to Core.

It isn't necessary to make the dev aware of that context to inform them of what the features are and how to use them, and it seems like the context will be less relevant in the future (2.1, 3.0, etc.). I don't recommend that it go in, but I'll see if I can explain that context out in the topic to address my own concerns about it (on Monday 😄).

This is beside the point but relevant to that context in a general way: I've favored the notion of two topics for many (but not all) subjects:

An introductory topic: Here are the features, and here's how to use them. Don't talk about how they work. Don't get into custom implementations. Go easy on history/migration notes (avoid them if possible). [My goal has been to write this type of topic for most of the topics I've produced in the past six weeks: This is what it does, and here's how the sample demo uses it. No mas! No mas!]
An advanced topic: Here's how the features work. Here are the migration notes for each release (link to the relevant GH issues/PRs). Here are design details. Here's how to replace OOB behaviors with your own implementations. Here's the roadmap for the next version. All of this other stuff that isn't required to know what the features are and how to use them would probably better serve the LOB dev if it were not in the main How-To introductory topic.

The single-topic alternative is to have the top-half be the introductory bits and the bottom-half be the advanced bits. When an LOB dev (or a newbie) arrives to learn what it is and how to use it, they probably only need to hit the top-half of the topic. Anywho ... just a thought ... while I'm on vacation. lol 😄

HaoK · 2017-11-22T00:10:40Z

Its fine to split things up where there's an introduction + advanced section. But in general we historically have very little technical documentation for options/auth/configuration, so that information needs to start going into some kind of official doc

HaoK · 2017-11-22T00:12:14Z

Basically ideally these docs are good for both introduction, and also as reference to developers who are extending/need to understand how/why things work the way they do

guardrex · 2017-11-28T01:00:54Z

@HaoK I made updates to address your last round of comments (in between 🚴 working off 🚴 outrageously good slices cherry pie from J's Pastry Shop in Pensacola 😄 ... J's cherry pie for 🎄??? ... YES! ... I think that's gonna happen! lol). I didn't change the current examples for IPostConfigureOptions because they match the sample in their current form. Other than that, I think the rest of the issues are addressed.

Update Updates Update metadata

HaoK · 2017-11-28T19:24:37Z

Cool, looks good

Rick-Anderson · 2017-11-28T22:07:38Z

aspnetcore/fundamentals/configuration/options.md

+delegate_option1 = value1_configured_by_delgate, delegate_option2 = 500
+```
+
+## Sub-options configuration


According to the Chicago Manual of Style, there should be no hyphenation after the prefix sub in words such as sub-heading.

Rick-Anderson · 2017-11-28T22:14:59Z

aspnetcore/fundamentals/configuration/options.md

+
+[!code-csharp[Main](options/sample/Startup.cs?name=snippet_Example3)]
+
+The `GetSection` extension method requires the [Microsoft.Extensions.Options.ConfigurationExtensions](https://www.nuget.org/packages/Microsoft.Extensions.Options.ConfigurationExtensions/) NuGet package. If the app already uses the [Microsoft.AspNetCore.All](https://www.nuget.org/packages/Microsoft.AspNetCore.All/) metapackage, the package is automatically included.


~~already~~ uses

Rick-Anderson · 2017-11-28T22:15:58Z

aspnetcore/fundamentals/configuration/options.md

+
+Options provided by a view model or with direct view injection is demonstrated as Example &num;4 in the [sample app](https://github.com/aspnet/docs/tree/master/aspnetcore/fundamentals/configuration/options/sample).
+
+Options can also be supplied in a view model or by injecting `IOptions<TOptions>` directly into a view (*Pages/Index.cshtml.cs*):


Options can ~~also~~ be supplied

Rick-Anderson · 2017-11-28T22:17:24Z

aspnetcore/fundamentals/configuration/options.md

+* The [Interface Segregation Principle (ISP)](http://deviq.com/interface-segregation-principle/): Features (classes) that depend on configuration settings depend only on the configuration settings that they use.
+* [Separation of Concerns](http://deviq.com/separation-of-concerns/): Settings for different parts of the app aren't dependent or coupled to one another.
+
+[View or download sample code](https://github.com/aspnet/docs/tree/master/aspnetcore/fundamentals/configuration/options/sample) ([how to download](xref:tutorials/index#how-to-download-a-sample))


I'd add.
This article is easier to follow with the downloaded sample.

Rick-Anderson · 2017-11-28T22:18:29Z

aspnetcore/fundamentals/configuration/options.md

+
+## Reload configuration data with IOptionsSnapshot
+
+Reload configuration data with `IOptionsSnapshot` as demonstrated in Example &num;5 in the [sample app](https://github.com/aspnet/docs/tree/master/aspnetcore/fundamentals/configuration/options/sample).


as demonstrated in
did you mean is demo'd?

It was to make "Reload" at the start of the sentence the verb (i.e., "[You] Reload ... as demonstrated ..."). I'm changing that now to make "is" the verb here to clean it up.

Rick-Anderson · 2017-11-28T22:24:10Z

aspnetcore/fundamentals/configuration/options.md

+
+*Requires ASP.NET Core 2.0 or later.*
+
+*Named options* support allows the app to distinguish between named options configurations. In the sample app, named options are declared with the [ConfigureNamedOptions&lt;TOptions&gt;.Configure](/dotnet/api/microsoft.extensions.options.configurenamedoptions-1.configure) method (*Startup.cs*):


remove (Startup.cs) everywhere.

Ok, but this was done to help ppl navigate in the sample ... tell them exactly which file to look at. I'll remove them.

Rick-Anderson · 2017-11-28T22:24:47Z

aspnetcore/fundamentals/configuration/options/sample/Pages/Index.cshtml.cs

+
+            #region snippet_Example6
+            // Example #6: Named options
+            var named_options_1 = $"named_options_1: option1 = {_named_options_1.Option1}, option2 = {_named_options_1.Option2}";


Rick-Anderson · 2017-11-28T22:25:52Z

aspnetcore/fundamentals/configuration/options.md

+named_options_2: option1 = named_options_2_value1_from_action, option2 = 5
+```
+
+`named_options_1` values are provided from configuration, which are loaded from the *appsettings.json* file. `named_options_2` values are provided by the `named_options_2` delegate in `ConfigureServices` for `Option1` and the default value for `Option2` provided by the `MyOptions` class.


named_options_2 values are provided by the named_options_2 delegate in ConfigureServices for Option1 and the default value for Option2 provided by the MyOptions class.

Too long.

Rick-Anderson · 2017-11-28T22:28:32Z

aspnetcore/fundamentals/configuration/options.md

+
+*Requires ASP.NET Core 2.0 or later.*
+
+Set post configuration with [IPostConfigureOptions&lt;TOptions&gt;](/dotnet/api/microsoft.extensions.options.ipostconfigureoptions-1) to run after all [IConfigureOptions&lt;TOptions&gt;](/dotnet/api/microsoft.extensions.options.iconfigureoptions-1) configuration occurs:


Set post configuration with IPostConfigureOptions<TOptions>. Post configuration runs after all IConfigureOptions<TOptions> configuration occurs:

... and coming off of the "sub" situation, "post" is a prefix here, so it should go "postconfiguration" in both spots.

Rick-Anderson · 2017-11-28T22:33:27Z

aspnetcore/fundamentals/configuration/options.md

+
+## Options factory, monitoring, and cache
+
+New in ASP.NET Core 2.0, [IOptionsFactory&lt;TOptions&gt;](/dotnet/api/microsoft.extensions.options.ioptionsfactory-1) is responsible for creating new options instances. It has a single [Create](/dotnet/api/microsoft.extensions.options.ioptionsfactory-1.create) method. The default implementation takes all registered `IConfigureOptions` and `IPostConfigureOptions` and runs all the configures first, followed by the post-configures. It distinguishes between `IConfigureNamedOptions` and `IConfigureOptions` and only calls the appropriate interface.


~~New in ASP.NET Core 2.0,~~
Requires in ASP.NET Core 2.0 or later.

I'll try a version on the next commit that has "(ASP.NET Core 2.0 or later)" after the interface.

Update

guardrex · 2017-11-28T23:21:48Z

@Rick-Anderson Feedback addressed. See lines 253, 255 on the use of "(ASP.NET Core 2.0 or later)" after the interface links.

See the way I broke the lines in ...

https://github.com/guardrex/Docs/blob/6c9d2be4d16e0b5a7925723dec4cfe652fed2a5e/aspnetcore/fundamentals/configuration/options/sample/Pages/Index.cshtml.cs

... or would you prefer that I add some IndexModel properties to split them into standalone statements?

For example, go from ...

public string SimpleOptionsWithDelegateConfig { get; private set; }
...
SimpleOptionsWithDelegateConfig = 
    $"delegate_option1 = {delegate_config_option1}, " +
    $"delegate_option2 = {delegate_config_option2}";

... to ...

public string SimpleOptionsWithDelegateConfig1 { get; private set; }
public string SimpleOptionsWithDelegateConfig2 { get; private set; }
...
SimpleOptionsWithDelegateConfig1 = $"delegate_option1 = {delegate_config_option1}";
SimpleOptionsWithDelegateConfig2 = $"delegate_option2 = {delegate_config_option2}";

guardrex changed the title ~~[WIP] Documenting Options change for 2.0~~ Documenting Options change for 2.0 Nov 15, 2017

Luke Latham added 2 commits November 20, 2017 11:17

Documenting Options change for 2.0

a9a6b65

Update Updates Updates

Updates

7314d8f

Fix link Update TOC entry Fix redo Updates

Update index.md

88aadff

Simple is banned.

Rick-Anderson self-assigned this Nov 20, 2017

Factor remaining Options out of Config topic

7c902c8

React to feedback and updates

4f2630d

Update Updates Update metadata

HaoK approved these changes Nov 28, 2017

View reviewed changes

Rick-Anderson requested changes Nov 28, 2017

View reviewed changes

React to feedback

6c9d2be

Update

Rick-Anderson approved these changes Nov 29, 2017

View reviewed changes

Rick-Anderson merged commit 374ecc3 into dotnet:master Nov 29, 2017

guardrex deleted the guardrex/options-update branch November 29, 2017 00:09

Rick-Anderson mentioned this pull request Nov 29, 2017

fundamentals/configuration - improve CSAT #4820

Closed

guardrex mentioned this pull request Nov 28, 2018

Emphasize IOptionsMonitor in Options topic + updates #9684

Merged


		[!code-csharp[Main](options/sample/Startup.cs?name=snippet_Example3)]

		The `GetSection` extension method requires the [Microsoft.Extensions.Options.ConfigurationExtensions](https://www.nuget.org/packages/Microsoft.Extensions.Options.ConfigurationExtensions/) NuGet package. If the app already uses the [Microsoft.AspNetCore.All](https://www.nuget.org/packages/Microsoft.AspNetCore.All/) metapackage, the package is automatically included.


		Options provided by a view model or with direct view injection is demonstrated as Example &num;4 in the [sample app](https://github.com/aspnet/docs/tree/master/aspnetcore/fundamentals/configuration/options/sample).

		Options can also be supplied in a view model or by injecting `IOptions<TOptions>` directly into a view (Pages/Index.cshtml.cs):


		## Reload configuration data with IOptionsSnapshot

		Reload configuration data with `IOptionsSnapshot` as demonstrated in Example &num;5 in the [sample app](https://github.com/aspnet/docs/tree/master/aspnetcore/fundamentals/configuration/options/sample).


		Requires ASP.NET Core 2.0 or later.

		Named options support allows the app to distinguish between named options configurations. In the sample app, named options are declared with the [ConfigureNamedOptions<TOptions>.Configure](/dotnet/api/microsoft.extensions.options.configurenamedoptions-1.configure) method (Startup.cs):


		Requires ASP.NET Core 2.0 or later.

		Set post configuration with [IPostConfigureOptions<TOptions>](/dotnet/api/microsoft.extensions.options.ipostconfigureoptions-1) to run after all [IConfigureOptions<TOptions>](/dotnet/api/microsoft.extensions.options.iconfigureoptions-1) configuration occurs:


		## Options factory, monitoring, and cache

		New in ASP.NET Core 2.0, [IOptionsFactory<TOptions>](/dotnet/api/microsoft.extensions.options.ioptionsfactory-1) is responsible for creating new options instances. It has a single [Create](/dotnet/api/microsoft.extensions.options.ioptionsfactory-1.create) method. The default implementation takes all registered `IConfigureOptions` and `IPostConfigureOptions` and runs all the configures first, followed by the post-configures. It distinguishes between `IConfigureNamedOptions` and `IConfigureOptions` and only calls the appropriate interface.

Documenting Options change for 2.0 #4797

Documenting Options change for 2.0 #4797

Conversation

guardrex commented Nov 15, 2017 • edited Loading

Notes

guardrex commented Nov 15, 2017

HaoK commented Nov 15, 2017

HaoK commented Nov 15, 2017

HaoK commented Nov 15, 2017 • edited Loading

guardrex commented Nov 15, 2017

HaoK commented Nov 15, 2017

guardrex commented Nov 15, 2017

Rick-Anderson commented Nov 15, 2017

HaoK commented Nov 15, 2017

guardrex commented Nov 15, 2017

guardrex commented Nov 15, 2017 • edited Loading

guardrex commented Nov 16, 2017

guardrex commented Nov 20, 2017

HaoK commented Nov 20, 2017

guardrex commented Nov 21, 2017

HaoK commented Nov 21, 2017

HaoK commented Nov 21, 2017

guardrex commented Nov 21, 2017

HaoK commented Nov 21, 2017

HaoK commented Nov 21, 2017

guardrex commented Nov 21, 2017 • edited Loading

HaoK commented Nov 22, 2017 • edited Loading

HaoK commented Nov 22, 2017

guardrex commented Nov 28, 2017

HaoK commented Nov 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guardrex commented Nov 28, 2017 • edited Loading

guardrex commented Nov 15, 2017 •

edited

Loading

HaoK commented Nov 15, 2017 •

edited

Loading

guardrex commented Nov 15, 2017 •

edited

Loading

guardrex commented Nov 21, 2017 •

edited

Loading

HaoK commented Nov 22, 2017 •

edited

Loading

guardrex commented Nov 28, 2017 •

edited

Loading