Skip to content

Small pieces of rename serialization cleanup#63066

Merged
CyrusNajmabadi merged 2 commits intodotnet:mainfrom
CyrusNajmabadi:renameOOPCleanup
Jul 29, 2022
Merged

Small pieces of rename serialization cleanup#63066
CyrusNajmabadi merged 2 commits intodotnet:mainfrom
CyrusNajmabadi:renameOOPCleanup

Conversation

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

Fallout after moving rename to have a more lightweight form of serialization when processing. Something i also want to pull out of a different, more complex, rename oop PR.

@ghost ghost added the Area-IDE label Jul 29, 2022

return result.Dehydrate();
var renameLocations = await SymbolicRenameLocations.FindLocationsInCurrentProcessAsync(
symbol, solution, options, GetClientOptionsProvider(callbackId), cancellationToken).ConfigureAwait(false);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

there was no point jumping to LightweightRenameLocations on the oop side. it exists on the host side as a temporary data so we can call back into oop efficiently without rehydrating symbols. THe previous code would make the LightweightRenameLocations then immediately convert that to SymbolicRenameLocations. so this just bypasses that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it possible we can annotate types that shouldn't be used in OOP/Not OOP locations based on folder paths? I assume outside of serialization everything in ServiceHub/* doesn't need to use serializable data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Definitely an good idea! :)


var locations = await LightweightRenameLocations.TryRehydrateAsync(
solution, GetClientOptionsProvider(callbackId), serializableLocations, cancellationToken).ConfigureAwait(false);
if (locations == null)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

similarly, we should not be going through LightweightRenameLocations on the oop side.

@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review July 29, 2022 20:06
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner July 29, 2022 20:06
@CyrusNajmabadi CyrusNajmabadi requested a review from dibarbet July 29, 2022 20:06
@CyrusNajmabadi CyrusNajmabadi enabled auto-merge July 29, 2022 20:12
@CyrusNajmabadi CyrusNajmabadi merged commit 279ed58 into dotnet:main Jul 29, 2022
@ghost ghost added this to the Next milestone Jul 29, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the renameOOPCleanup branch July 29, 2022 21:52
@dibarbet dibarbet modified the milestones: Next, 17.4 P2 Sep 1, 2022
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.

3 participants