Fix a couple of cases where feature options were not being hooked up.#66241
Fix a couple of cases where feature options were not being hooked up.#66241CyrusNajmabadi merged 3 commits intodotnet:mainfrom
Conversation
|
possibly. @tmat for ideas. Generally speaking though, i'm not a fan of these option types necessarily having optional values. If we just made them required it would ensure that they had to be properly updated when things changed. |
|
We have a test that validates this, it's just missing this particular option type: @CyrusNajmabadi Could you add the options to the test, run w/o your change and see if it fails?
Unfortunately, that would also mean that you'd need to specify all the options every time. Some of these types have dozens of options, so that's very inconvenient to use if you just need to change a couple of options. Some of these types are only ever initialized from global options, so perhaps they can have all the properties required. But the plan is to make some of these option types public (to replace OptionSet, for example in Formatter, Simplifier, and other APIs) and we definitely don't want to require all the properties specified in that case. |
|
@tmat Yup, that's tough. I don't have a godo answer here. I'll update the test though! |
|
The test is the best solution I could find. In principle, we want to mark the call-site as requiring all arguments. A special comment and analyzer would be an option... |
No description provided.