8000 Fix stylecop hungarian by iSazonov · Pull Request #9281 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Fix stylecop hungarian #9281

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

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

Conversation

@iSazonov
Copy link
Collaborator
@iSazonov iSazonov commented Apr 3, 2019

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
Member

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

@iSazonov
Copy link
Collaborator Author
iSazonov commented Apr 4, 2019

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
Member

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

@SteveL-MSFT
Copy link
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
Copy link
Collaborator Author
iSazonov commented Apr 5, 2019

@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
Copy link
Collaborator Author
iSazonov commented Apr 5, 2019

@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

0