Skip to content
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

Switch SG mode to 'balanced' by default #73618

Merged
merged 13 commits into from
May 22, 2024

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels May 21, 2024
@@ -50,5 +50,5 @@ public static WorkspaceConfigurationOptions GetWorkspaceConfigurationOptions(thi
SourceGeneratorExecutionPreferenceUtilities.GetEditorConfigString));

public static readonly Option2<bool> SourceGeneratorExecutionBalancedFeatureFlag = new(
"dotnet_source_generator_execution_balanced_feature_flag", false);
"dotnet_source_generator_execution_balanced_feature_flag", true);
Copy link
Member Author

Choose a reason for hiding this comment

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

note: we still have the feature flag. So we still have control-tower support to flip this back to 'false' if we run into any problems with this in the wild.

Copy link
Member

Choose a reason for hiding this comment

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

Think we might need to have a modification here to leave it false by default for VSCode - I haven't yet had a chance to implement balanced mode there yet. Easiest way if you want to change the default is have a line of code in the language server project to explicitly set this to false - https://github.com/dotnet/roslyn/blob/main/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs#L90

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review May 21, 2024 21:09
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner May 21, 2024 21:09
@CyrusNajmabadi CyrusNajmabadi enabled auto-merge May 21, 2024 21:59

// If we added or removed analyzers, then re-run all generators to bring them up to date.
if (hasAnalyzerChanges)
_projectSystemProjectFactory.Workspace.EnqueueUpdateSourceGeneratorVersion(projectId: null, forceRegeneration: true);
Copy link
Member

Choose a reason for hiding this comment

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

Why is projectId null here?

Copy link
Member Author

Choose a reason for hiding this comment

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

that means "redo all generators".

Copy link
Member

Choose a reason for hiding this comment

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

@CyrusNajmabadi For all projects though?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes.

Optional severity As RenameDashboardSeverity = RenameDashboardSeverity.None
) As Tasks.Task
Optional severity As RenameDashboardSeverity = RenameDashboardSeverity.None,
Optional executionPreference As SourceGeneratorExecutionPreference = SourceGeneratorExecutionPreference.Automatic) As Task
Copy link
Member

Choose a reason for hiding this comment

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

It feels a bit odd to me that rename tests are concerning themselves with this -- it seems like this is just something the underlying system should be handling. But maybe I'm missing something here since I'm ramping back up on this space.

@CyrusNajmabadi CyrusNajmabadi merged commit a1576d6 into dotnet:main May 22, 2024
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone May 22, 2024
@CyrusNajmabadi CyrusNajmabadi deleted the sourceGenBalanced branch May 22, 2024 00:41
Cosifne added a commit that referenced this pull request May 23, 2024
Cosifne added a commit that referenced this pull request May 23, 2024
@Cosifne Cosifne modified the milestones: Next, 17.11 P2 May 28, 2024
Cosifne added a commit to Cosifne/roslyn that referenced this pull request Jun 6, 2024
…Balanced"

This reverts commit a1576d6, reversing
changes made to 45cb0c8.
CyrusNajmabadi added a commit to CyrusNajmabadi/roslyn that referenced this pull request Jun 6, 2024
…Balanced"

This reverts commit a1576d6, reversing
changes made to 45cb0c8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants