-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Remove public facing OOP options. #77711
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
Remove public facing OOP options. #77711
Conversation
| {"dotnet_enable_event_hook_up", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "Event Hookup")}, | ||
| {"dotnet_format_on_save", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "FormatOnSave")}, | ||
| {"dotnet_code_analysis_in_separate_process", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "OOP64Bit")}, | ||
| {"dotnet_code_analysis_in_separate_process", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "OOP64Bit_New")}, |
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.
these flags still exist as an unsupported way to opt out of these features. however, i've moved them to new regkeys so that everyone defualts into the supported state. that way someone who disabled something in the past, isn't able to reenable if they want that.
genlu
left a comment
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.
![]()
| {"dotnet_enable_event_hook_up", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "Event Hookup")}, | ||
| {"dotnet_format_on_save", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "FormatOnSave")}, | ||
| {"dotnet_code_analysis_in_separate_process", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "OOP64Bit")}, | ||
| {"dotnet_code_analysis_in_separate_process", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "DotnetCodeAnalysisInSeparateProcess")}, |
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.
these flags still exist as an unsupported way to opt out of these features. however, i've moved them to new regkeys so that everyone defualts into the supported state. that way someone who disabled something in the past, isn't able to reenable if they want that.
jasonmalinowski
left a comment
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.
![]()
| SourceGeneratorExecution: globalOptions.GetOption(SourceGeneratorExecution), | ||
| ReloadChangedAnalyzerReferences: | ||
| globalOptions.GetOption(ReloadChangedAnalyzerReferences) ?? globalOptions.GetOption(ReloadChangedAnalyzerReferencesFeatureFlag), | ||
| globalOptions.GetOption(ReloadChangedAnalyzerReferences), |
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.
Should we just pull this entirely? Whereas the in-proc/out-of-proc can have a bit of user impact if they depend on it in obscure ways, this is only needed if we have bugs, right?
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.
yes. but if there is a bug, i don't mind having a reg-key to disable this if a big customer needs it.
| {"dotnet_enable_event_hook_up", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "Event Hookup")}, | ||
| {"dotnet_format_on_save", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "FormatOnSave")}, | ||
| {"dotnet_code_analysis_in_separate_process", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "OOP64Bit")}, | ||
| {"dotnet_code_analysis_in_separate_process", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "DotnetCodeAnalysisInSeparateProcess")}, |
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.
| {"dotnet_code_analysis_in_separate_process", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "DotnetCodeAnalysisInSeparateProcess")}, | |
| {"dotnet_code_analysis_in_separate_process", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "CodeAnalysisInSeparateProcess")}, |
The "dotnet" prefix isn't really needed for the regkey since it's already Roslyn prefixed. Not worth changing if you don't have to touch this PR for some other reason but I'm generally but not sure we want to start a tradition of prefixing everything redundantly.
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.
do. the idea for me was to keep teh name of the value in line with the snake_cased_name. so it's all 'consistent' in that way.
f518467
into
dotnet:release/dev17.15
We will still have reg keys available if absolutely necessary. But this will no longer be a supported config as we need OOP for several core capabilities (reloading generators/analyzers, Razor, semantic search and others).