-
Notifications
You must be signed in to change notification settings - Fork 199
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
Clean up code actions code #10582
Clean up code actions code #10582
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some thoughts, but overall like this change. Thanks for cleaning things up :) I didn't check code that was far outside of changed reasons
...ft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/TypeAccessibilityCodeActionProvider.cs
Show resolved
Hide resolved
...c/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/AddUsingsCodeActionResolver.cs
Outdated
Show resolved
Hide resolved
...spNetCore.Razor.LanguageServer/CodeActions/Razor/ComponentAccessibilityCodeActionProvider.cs
Outdated
Show resolved
Hide resolved
...rosoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/GenerateMethodCodeActionResolver.cs
Show resolved
Hide resolved
src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/RazorCodeActionContext.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CodeActionEndpoint.cs
Show resolved
Hide resolved
...t.AspNetCore.Razor.LanguageServer/CodeActions/Razor/ExtractToCodeBehindCodeActionResolver.cs
Show resolved
Hide resolved
...rosoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/GenerateMethodCodeActionProvider.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a good topic for Monday codeabout - just talk through the list of (informal?) conventions we have to make it more explicit (unless there are more pressing topics)
LOVE this idea. |
...rosoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/GenerateMethodCodeActionResolver.cs
Show resolved
Hide resolved
...rosoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/GenerateMethodCodeActionResolver.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it! Many welcome changes here. We can always argue over minor details, but the overall code improvement in this pull request is indisputable.
This PR is a style-only clean up of our code actions handlers and related code. Definitely could have gone further (more collection pooling, returning concrete types etc.) but I wanted to try to have it serve as an indication for what we consider our current idiomatic coding style* and to avoid conflicts with the extract to component work.
* Current style is subject to change at any time, and are not well documented. To quote a great Australian film, it's just the vibe.