Skip to content

Reformat code according to EditorConfig rules - #11681

Merged
Travis Plunk (TravisEz13) merged 2 commits into
PowerShell:masterfrom
xtqqczze:patch-dotnet-format
Jan 31, 2020
Merged

Reformat code according to EditorConfig rules#11681
Travis Plunk (TravisEz13) merged 2 commits into
PowerShell:masterfrom
xtqqczze:patch-dotnet-format

Conversation

@xtqqczze

@xtqqczze xtqqczze commented Jan 26, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Summary

Reformat source code with dotnet/format according to rules in the .editorconfig file.

dotnet tool install --global dotnet-format --version 3.1.37601
dotnet-format

The following rules were disabled as they resulted in a high number of changes:

csharp_preserve_single_line_statements = false
csharp_new_line_before_open_brace = all

To exclude regressions, we could create a new static test:

# Terminates with a non-zero exit code if any files were formatted
dotnet-format --check --dry-run

PR Context

PR Checklist

@xtqqczze xtqqczze mentioned this pull request Jan 26, 2020
Merged
14 tasks
iSazonov
Ilya (iSazonov) reviewed Jan 26, 2020
View reviewed changes

@iSazonov Ilya (iSazonov) left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

xtqqczze Please revert changes in .editorconfig. They was needed only locally to allow you to format but we should keep these options as they correspond to our code convention.
(For static test we could use another config file with fewer rules.)

Which rules would you like to see added?

I see many rules in .Net Core and Roslyn .editorconfig-s and we could review them and add some ones to our config. When I added these rules last time I had to remove many because there were a lot of problems. Now the code is cleaner and we can add more rules.

Also please see CodeFactor issues. We need to adjust CodeFactor or formatting options to resolve them.

@xtqqczze

xtqqczze commented Jan 26, 2020

Copy link
Copy Markdown
Contributor Author

CodeFactor issues are violations of SA1008 due to added space after the delegate keyword. Rule documentation states an opening parenthesis can be preceded by whitespace when it is preceded by "certain C# keywords" but is ambiguous as to whether or not this applies to the delegate keyword. There is an open issue dotnet/roslyn#3257 concerning this behaviour.

We can apply a refactoring to replace the anonymous methods with delegate keyword with lamba expressions and avoid the rule violations. This would be a workaround but is also a recommended refactoring.

@xtqqczze xtqqczze mentioned this pull request Jan 27, 2020
Merged
14 tasks
@xtqqczze

xtqqczze commented Jan 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) This PR disables those rules because it is apparent the code is not already in the prescribed style. We could open a new issue to discuss which rules we want for our code convention?

@xtqqczze
xtqqczze force-pushed the patch-dotnet-format branch from 75aef8b to 3052d56 Compare January 27, 2020 03:07
@xtqqczze

xtqqczze commented Jan 27, 2020

Copy link
Copy Markdown
Contributor Author

rebased to restart CI

@iSazonov

Ilya (iSazonov) commented Jan 27, 2020

Copy link
Copy Markdown
Collaborator

This PR disables those rules because it is apparent the code is not already in the prescribed style.

We have over 50000 CodeFactor issues but we do not disable the rules because we don't want add new issues.

We could open a new issue to discuss which rules we want for our code convention?

We already have Code Conventions. This allows us add any rules to suggest/force the Code Conventions.

@xtqqczze xtqqczze changed the title [WIP] Reformat code with dotnet-format according to EditorConfig rules Reformat code according to EditorConfig rules Jan 27, 2020
@iSazonov

Ilya (iSazonov) commented Jan 30, 2020

Copy link
Copy Markdown
Collaborator

xtqqczze Please resolve merge conflict.

@xtqqczze
xtqqczze force-pushed the patch-dotnet-format branch from 18570c9 to e2fea92 Compare January 30, 2020 10:16
@xtqqczze

xtqqczze commented Jan 30, 2020

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) rebased and removed commit "Remove duplicate semicolons" This should be part of a seperate PR as the change is unrelated to using the dotnet-format tool.

@xtqqczze

xtqqczze commented Jan 30, 2020

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) We should make changes to rules in .editorconfig before creating a new dotnet-format PR.

@iSazonov Ilya (iSazonov) added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Jan 30, 2020
@iSazonov Ilya (iSazonov) added this to the 7.1.0-preview.1 milestone Jan 30, 2020
@TravisEz13

Travis Plunk (TravisEz13) commented Jan 30, 2020

Copy link
Copy Markdown
Member

Please resolve merge conflicts.
Such a large PR may be difficult to get merged.

@xtqqczze
xtqqczze force-pushed the patch-dotnet-format branch from e2fea92 to c8ffda3 Compare January 30, 2020 21:15
@xtqqczze

xtqqczze commented Jan 30, 2020

Copy link
Copy Markdown
Contributor Author

Travis Plunk (@TravisEz13) rebased

@xtqqczze
xtqqczze force-pushed the patch-dotnet-format branch from c8ffda3 to edab15a Compare January 30, 2020 23:15
@xtqqczze

xtqqczze commented Jan 30, 2020

Copy link
Copy Markdown
Contributor Author

Travis Plunk (@TravisEz13) rebased, again

iSazonov
Ilya (iSazonov) reviewed Jan 31, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/debugger/debugger.cs Outdated
@xtqqczze
xtqqczze force-pushed the patch-dotnet-format branch from edab15a to 83106fd Compare January 31, 2020 11:18
@xtqqczze

xtqqczze commented Jan 31, 2020

Copy link
Copy Markdown
Contributor Author

PowerShell-CI-static-analysis failures are in markdown-links and are unrelated.

@TravisEz13
Travis Plunk (TravisEz13) merged commit a34d0f3 into PowerShell:master Jan 31, 2020
@xtqqczze
xtqqczze deleted the patch-dotnet-format branch February 1, 2020 12:23
@ghost

Deleted user (ghost) commented Mar 26, 2020

Copy link
Copy Markdown

🎉v7.1.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

@xtqqczze xtqqczze mentioned this pull request Aug 22, 2020
Merged
8 tasks
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
@xtqqczze
Reformat code according to EditorConfig rules (PowerShell#11681)
e7e0158
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov Ilya (iSazonov) iSazonov approved these changes
@TravisEz13 Travis Plunk (TravisEz13) TravisEz13 approved these changes
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
@anmenaga Andrew (anmenaga) Awaiting requested review from anmenaga
@daxian-dbw Dongbo Wang (daxian-dbw) Awaiting requested review from daxian-dbw daxian-dbw is a code owner
@SteveL-MSFT Steve Lee (SteveL-MSFT) Awaiting requested review from SteveL-MSFT

Assignees

@TravisEz13 Travis Plunk (TravisEz13)

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Milestone

7.1.0-preview.1

Development

Successfully merging this pull request may close these issues.

3 participants

@xtqqczze @iSazonov @TravisEz13