Skip to content

Add feature flag so we can remotely disable operator-completion.#52002

Merged
CyrusNajmabadi merged 3 commits intodotnet:mainfrom
CyrusNajmabadi:completionAB
Mar 20, 2021
Merged

Add feature flag so we can remotely disable operator-completion.#52002
CyrusNajmabadi merged 3 commits intodotnet:mainfrom
CyrusNajmabadi:completionAB

Conversation

@CyrusNajmabadi
Copy link
Contributor

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner March 19, 2021 22:29
@ghost ghost added the Area-IDE label Mar 19, 2021
@CyrusNajmabadi CyrusNajmabadi requested a review from genlu March 19, 2021 22:29
public const string LspTextSyncEnabled = "Roslyn.LspTextSyncEnabled";
public const string RemoveUnusedReferences = "Roslyn.RemoveUnusedReferences";
public const string LSPCompletion = "Roslyn.LSP.Completion";
public const string UnnamedSymbolCompletionDisabled = "Roslyn.UnnamedSymbolCompletionDisabled";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that this is a negative option shouldn't it be Roslyn.UnnamedSymbolCompletion whose default is true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a feature flag. we can't make a default for it. that would force us to have the experiment always running for all time, in order for this to work.

So this is the negative so we can say: by default, this runs. but an experiment can be enabled to turn it off. As opposed to: this is off, and we must always run an experiment forever to turn it on.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add handling for this in VisualStudioExperimentationService for this but that does seem like overkill I suppose. We own all implementations of IExperimentationService so it's up to us to decide what an option being enabled means.

@CyrusNajmabadi CyrusNajmabadi requested a review from jmarolf March 19, 2021 22:54
@CyrusNajmabadi CyrusNajmabadi merged commit 4a9d841 into dotnet:main Mar 20, 2021
@CyrusNajmabadi CyrusNajmabadi deleted the completionAB branch March 20, 2021 09:13
@ghost ghost added this to the Next milestone Mar 20, 2021
// Escape hatch feature flag to let us disable this feature remotely if we run into any issues with it,
var workspace = document.Project.Solution.Workspace;
var experimentationService = workspace.Services.GetRequiredService<IExperimentationService>();
var disabled = experimentationService.IsExperimentEnabled(WellKnownExperimentNames.UnnamedSymbolCompletionDisabled);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗ This needs an option as well so it can be manually enabled via vsregedit. Here's an example from another feature:

return services.GetRequiredService<IOptionService>().GetOption(OOPServerGC)
|| services.GetService<IExperimentationService>()?.IsExperimentEnabled(WellKnownExperimentNames.OOPServerGC) == true;

@allisonchou allisonchou modified the milestones: Next, 16.10.P2 Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants