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

Merge the new UnitTesting solution crawler into Main #64368

Merged
merged 199 commits into from
Oct 6, 2022

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

CyrusNajmabadi and others added 14 commits September 29, 2022 15:42
Remove unneeded SolutionCrawler reasons that UnitTesting is not using.
…/RemoteLegacySolutionEventsAggregationService.cs
…/RemoteLegacySolutionEventsAggregationService.cs
…nEvents

Merge main to features/solutionEvents
…sWorkspaceEventListener.cs

Co-authored-by: Gen Lu <genlu@users.noreply.github.com>
Wait until an analyzer is registered before streaming workspace events over to OOP
…nEvents

Merge main to features/solutionEvents
…nEvents

Merge main to features/solutionEvents
…nEvents

Merge main to features/solutionEvents
…nEvents

Merge main to features/solutionEvents
@CyrusNajmabadi
Copy link
Member Author

@tmat Please review. This is merging all the solution-crawler work done in the feature branch back into main :)

…nEvents

Merge main to features/solutionEvents
@CyrusNajmabadi
Copy link
Member Author

@tmat review please :)

private InvocationReasons(ImmutableHashSet<string> reasons)
=> _reasons = reasons;
public InvocationReasons(ImmutableHashSet<string> reasons)
=> _reasons = reasons ?? ImmutableHashSet<string>.Empty;
Copy link
Member

Choose a reason for hiding this comment

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

reasons

Can this actually be null? There is no ? in the signature.

@@ -19,8 +19,8 @@ namespace Microsoft.CodeAnalysis.ExternalAccess.UnitTesting.Api
public static readonly UnitTestingInvocationReasonsWrapper PredefinedSyntaxChanged = new(PredefinedInvocationReasons.SyntaxChanged);
public static readonly UnitTestingInvocationReasonsWrapper PredefinedProjectConfigurationChanged = new(PredefinedInvocationReasons.ProjectConfigurationChanged);
public static readonly UnitTestingInvocationReasonsWrapper PredefinedDocumentOpened = new(PredefinedInvocationReasons.DocumentOpened);
public static readonly UnitTestingInvocationReasonsWrapper PredefinedDocumentRemoved = new(PredefinedInvocationReasons.DocumentRemoved);
public static readonly UnitTestingInvocationReasonsWrapper PredefinedDocumentClosed = new(PredefinedInvocationReasons.DocumentClosed);
Copy link
Member

Choose a reason for hiding this comment

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

readonly

Nit: no change here, just reodering

bool ShouldReportChanges(SolutionServices services);

ValueTask OnWorkspaceChangedAsync(WorkspaceChangeEventArgs args, CancellationToken cancellationToken);
#if false // Not used in unit testing crawling
Copy link
Member

Choose a reason for hiding this comment

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

false

Why keep this?

{
internal interface INewUnitTestingIncrementalAnalyzerImplementation
{
#if false // Not used in unit testing crawling
Copy link
Member

Choose a reason for hiding this comment

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

false

Why keep this?

Copy link
Member

@tmat tmat left a comment

Choose a reason for hiding this comment

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

:shipit:

@CyrusNajmabadi CyrusNajmabadi merged commit 957fa61 into main Oct 6, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the features/solutionEvents branch October 6, 2022 18:38
@ghost ghost added this to the Next milestone Oct 6, 2022
@CyrusNajmabadi
Copy link
Member Author

@shyamnamboodiripad this is now merged. PTAL when you can and see about moving onto this :) LMK if you run into any issues. Thanks!

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.

5 participants