Skip to content

Run naming styles on closed files#37287

Merged
jasonmalinowski merged 2 commits intodotnet:masterfrom
jasonmalinowski:run-naming-styles-on-closed-files
Feb 4, 2020
Merged

Run naming styles on closed files#37287
jasonmalinowski merged 2 commits intodotnet:masterfrom
jasonmalinowski:run-naming-styles-on-closed-files

Conversation

@jasonmalinowski
Copy link
Copy Markdown
Member

@jasonmalinowski jasonmalinowski commented Jul 16, 2019

Our best guess is we ran naming styles on open files only for two reasons:

  1. At the time, we only supported .editorconfig in-proc, so out of proc wouldn't have used the right settings.
  2. Even if we were running in-proc, there might have been performance issues since we were running across a lot of files.

Neither of these concerns apply at this point: we now support .editorconfig out of proc (both in the legacy path and the new path) and since we're already running other analyzers that use .editorconfig,
the cost of processing it will exist regardless.

Fixes #15760

@jasonmalinowski jasonmalinowski requested a review from a team as a code owner July 16, 2019 23:08
Comment thread src/Workspaces/Core/Portable/Execution/AbstractOptionsSerializationService.cs Outdated
Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

Approve
Submit feedback approving these changes.

@jasonmalinowski jasonmalinowski self-assigned this Jul 17, 2019
@jinujoseph jinujoseph added this to the 16.3.P2 milestone Jul 18, 2019
@jasonmalinowski jasonmalinowski modified the milestones: 16.3.P2, 16.4 Sep 6, 2019
@mavasani
Copy link
Copy Markdown
Contributor

@jasonmalinowski Any progress on this one? There is a VS feedback user request to view all naming violations in a project/solution which depends on this PR.

@jinujoseph jinujoseph modified the milestones: 16.4, 16.5.P2 Dec 11, 2019
@mavasani
Copy link
Copy Markdown
Contributor

Another VS feedback requesting naming violations on closed files: https://developercommunity.visualstudio.com/content/problem/880060/vs2019-naming-rules-applied-inconsistently.html

@jasonmalinowski Any update on this PR?

@jasonmalinowski jasonmalinowski modified the milestones: 16.5.P2, 16.5.P3 Jan 17, 2020
@jasonmalinowski jasonmalinowski force-pushed the run-naming-styles-on-closed-files branch 3 times, most recently from 1356223 to 618598a Compare January 31, 2020 22:26
Our best guess is we ran naming styles on open files only for two
reasons:

1. At the time, we only supported .editorconfig in-proc, so
   out of proc wouldn't have used the right settings.
2. Even if we were running in-proc, there might have been performance
   issues since we were running across a lot of files.

Neither of these concerns apply at this point: we now support
.editorconfig out of proc (both in the legacy path and the new path) and
since we're already running other analyzers that use .editorconfig,
the cost of processing it will exist regardless.
@jasonmalinowski jasonmalinowski force-pushed the run-naming-styles-on-closed-files branch from 618598a to b4cc326 Compare January 31, 2020 22:33
@jasonmalinowski
Copy link
Copy Markdown
Member Author

RPS run passed internally, which is what @sharwell suspected since this is unlikely running in that scenario in the first place.

@jasonmalinowski
Copy link
Copy Markdown
Member Author

@mavasani and @sharwell any concerns prior to merge? Your signoffs are ancient at this point but nothing really changed.

@mavasani
Copy link
Copy Markdown
Contributor

mavasani commented Feb 4, 2020

Nope, looks all good to merge. Thanks Jason!

@jasonmalinowski jasonmalinowski merged commit c3198fd into dotnet:master Feb 4, 2020
@jasonmalinowski jasonmalinowski deleted the run-naming-styles-on-closed-files branch February 4, 2020 01:56
@davkean
Copy link
Copy Markdown
Member

davkean commented Feb 4, 2020

OMG - thank-you thank-you thank-you!

@sharwell
Copy link
Copy Markdown
Contributor

sharwell commented Feb 4, 2020

So glad this made the cutoff!

@mavasani
Copy link
Copy Markdown
Contributor

mavasani commented Feb 4, 2020

You should also be able to get advantage of this feature with "Open Document" analysis scope + invoking "Run Code Analysis" command on project/solution for an demand explicit execution.

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.

The Error List shows different results based on whether you've ever opened a source file

6 participants