Skip to content

Fix stylecop hungarian #9281

Merged
TravisEz13 merged 4 commits into
PowerShell:masterfrom
iSazonov:fix-stylecop-hungarian-
Apr 5, 2019
Merged

Fix stylecop hungarian #9281
TravisEz13 merged 4 commits into
PowerShell:masterfrom
iSazonov:fix-stylecop-hungarian-

Conversation

@iSazonov

@iSazonov iSazonov commented Apr 3, 2019

Copy link
Copy Markdown
Collaborator

PR Summary

Add very popular "is" and "_is" prefixes.

PR Context

PR Checklist

@iSazonov iSazonov added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Apr 3, 2019
@iSazonov iSazonov added this to the 6.3.0-preview.1 milestone Apr 3, 2019
@iSazonov iSazonov self-assigned this Apr 3, 2019
@SteveL-MSFT

Copy link
Copy Markdown
Member

Should we just enable allowCommonHungarianPrefixes? It seems that you don't need to explicitly specify the leading underscore version?

@iSazonov

iSazonov commented Apr 4, 2019

Copy link
Copy Markdown
Collaborator Author

Yes, but CodeFactor still reports "is" and "_is" https://www.codefactor.io/repository/github/powershell/powershell/issues?category=Style&groupId=15

Update: from previous PR https://www.codefactor.io/repository/github/powershell/powershell/pull/8500 - there is CodeFactor reports that "is" and "_is" too.
I do not understand where the problem is :-(

@TravisEz13

Copy link
Copy Markdown
Member

some tools read from master and don't use the configuration files from the PR. I'm just guessing.

@SteveL-MSFT

Copy link
Copy Markdown
Member

It's actually difficult to read the updated config from a PR as the json you get back is just the diff of the file and not the complete file. This is why for @PoshChan I always look in master rather than trying to generate the resulting config file from master + diff.

@iSazonov

iSazonov commented Apr 5, 2019

Copy link
Copy Markdown
Collaborator Author

@TravisEz13 There is really a problem. As @SteveL-MSFT mentioned above allowCommonHungarianPrefixes enabled "is" by default - I found this in source code too. But CodeFactor continues to report "is". We can see this in previous @daxian-dbw PR #8500 - it seems CodeFactor read config from commit - and then in master too.

@iSazonov

iSazonov commented Apr 5, 2019

Copy link
Copy Markdown
Collaborator Author

@SteveL-MSFT @TravisEz13 @daxian-dbw

I received a response from the CodeFactor support:

Thank you for reaching out.
CodeFactor supports Settings.StyleCop file as config, but not stylecop.json (it will be supported once https://github.com/DotNetAnalyzers/StyleCopAnalyzers/releases is out of RC).
So, you can still configure StyleCop to do what allowCommonHungarianPrefixes proposes by modifying Settings.StyleCop file (e.g. https://stackoverflow.com/a/45606734)
Please let me know if I was able to address your inquiry or if you need anything else.
Thank you,
Sandra Jones
Your CodeFactor.io team
Track CodeFactor.io feature announcements on Twitter https://twitter.com/CodeFactor_io

So I reverted previous commit and change Settings.StyleCop as recommended. You can see that now CodeFactor reports "2253 issues fixed."

We could move stylecop.json to Tools folder and use Settings.StyleCop until CodeFactor get new StyleCopAnalyzers version.

@TravisEz13 TravisEz13 merged commit 6118c37 into PowerShell:master Apr 5, 2019
@iSazonov iSazonov deleted the fix-stylecop-hungarian- branch April 5, 2019 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants