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.

Sign up for GitHub

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

Jump to bottom

Implement support for CodeRefactoringProvider in NuGet packages #35685

Merged
merged 5 commits into from
Jan 4, 2020
Merged

Implement support for CodeRefactoringProvider in NuGet packages #35685

merged 5 commits into from
Jan 4, 2020

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented May 13, 2019 •
edited
Loading

Fixes #32705

@sharwell sharwell requested a review from a team as a code owner May 13, 2019 11:59
CyrusNajmabadi
CyrusNajmabadi reviewed May 13, 2019
View reviewed changes
src/Features/Core/Portable/CodeRefactorings/CodeRefactoringService.cs
return builder.ToImmutable();
}

private class ProjectCodeRefactoringProvider
Copy link
Member

Choose a reason for hiding this comment

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

nit: consider extracting to its own file.
nit: i'm on the fence if 'Project' or 'AnalyzerReference' is preferred in the name here. I kinda like AnalyzerReferenceCodeRefactoringsProvider.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copied the pattern from CodeFixService. One option is to make a follow-up to change both to something else.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we extract most of the common functionality between this class and ProjectCodeFixProvider into a common generic base type.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed. looks very shareable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Leaving this for a follow-up

@CyrusNajmabadi
Copy link
Member

Overall i really like this. Some small nits/suggestions! :)

@sharwell sharwell mentioned this pull request Jun 6, 2019
Merged
@mavasani mavasani mentioned this pull request Jun 17, 2019
Merged
@jcouv
Copy link
Member

jcouv commented Jul 8, 2019

Any luck on pushing this PR forward? I'm hoping to add a refactoring that will be useful to the compiler team (#2588, depends on this PR).

@mavasani
Copy link
Contributor

mavasani commented Jul 8, 2019

@jcouv A quicker approach might be to change the refactoring into a pair of analyzer/fixer with hidden diagnostics. This will also provide FixAll support which is still missing for refactorings.

@jcouv
Copy link
Member

jcouv commented Oct 17, 2019

@sharwell Any luck pushing this PR forward? Are there any major issues to be solved, or just feedback to address?

@jcouv
Copy link
Member

jcouv commented Nov 21, 2019

@sharwell Any luck pushing this PR forward?

1 similar comment
@jcouv
Copy link
Member

jcouv commented Dec 20, 2019

@sharwell Any luck pushing this PR forward?

@sharwell
Copy link
Member Author

@jcouv I was working to refactor the services to support completion providers as well. The initial refactoring to simplify completion provider registration is waiting for VS for Mac validation.

@sharwell sharwell merged commit 999ce12 into dotnet:master Jan 4, 2020
@sharwell sharwell deleted the refactorings branch January 4, 2020 08:37
@sharwell sharwell added this to the 16.5.P2 milestone Jan 4, 2020
@ericsampson ericsampson mentioned this pull request Apr 16, 2020
Closed
@Looka997 Looka997 mentioned this pull request Jun 30, 2023
Closed
@dibarbet dibarbet mentioned this pull request Jan 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi left review comments

@genlu genlu genlu left review comments

@Therzok Therzok Therzok left review comments

@mavasani mavasani mavasani approved these changes

Assignees
No one assigned
Labels
Area-Analyzers Feature Request
Projects
None yet
Milestone
16.5.P2
Development

Successfully merging this pull request may close these issues.

CodeRefactoring in nuget package doesn't work
7 participants
@sharwell @CyrusNajmabadi @jcouv @mavasani @Therzok @genlu @jinujoseph