Skip to content

Move the 'finding' portoin of rename out of process.#43554

Merged
16 commits merged intodotnet:masterfrom
CyrusNajmabadi:renameOOP4
Apr 22, 2020
Merged

Move the 'finding' portoin of rename out of process.#43554
16 commits merged intodotnet:masterfrom
CyrusNajmabadi:renameOOP4

Conversation

@CyrusNajmabadi
Copy link
Contributor

This PR should be reviewed commit by commit.

Rename is composed of two parts. The first finds the all the places to rename (and only needs to run once). The second checks for conflicts at all those locations whenever the user updates the name (so it may run many times). This PR moves the first part of this out of process.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 22, 2020 01:44
@CyrusNajmabadi CyrusNajmabadi requested review from a team, jasonmalinowski and tmat April 22, 2020 01:44
throw new ArgumentException(WorkspacesResources.Symbols_project_could_not_be_found_in_the_provided_solution, nameof(symbol));

if (string.IsNullOrEmpty(newName))
throw new ArgumentException(nameof(newName));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

public methods get real argument checks. internal methods get contract checks.

Copy link
Contributor

@ryzngard ryzngard left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@ghost ghost merged commit a1b8670 into dotnet:master Apr 22, 2020
@jinujoseph jinujoseph added this to the Next milestone Apr 22, 2020
@CyrusNajmabadi CyrusNajmabadi deleted the renameOOP4 branch April 23, 2020 04:37
@sharwell sharwell modified the milestones: Next, temp, 16.7.P1 Apr 28, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants