Skip to content

Fixes of around 350 PSScriptAnalyzer warnings#4213

Closed
bergmeister wants to merge 4 commits intoPowerShell:masterfrom
bergmeister:PSScriptAnalyzerWarningFixesAfterRebasedCommits
Closed

Fixes of around 350 PSScriptAnalyzer warnings#4213
bergmeister wants to merge 4 commits intoPowerShell:masterfrom
bergmeister:PSScriptAnalyzerWarningFixesAfterRebasedCommits

Conversation

@bergmeister
Copy link
Copy Markdown
Contributor

@bergmeister bergmeister commented Jul 10, 2017 •
edited
Loading

This is the clean and rebased version with the split into 4 separate commits as requested by the original PR #4205
Most of them are fixes of PSPossibleIncorrectComparisonWithNull, PSAvoidUsingCmdletAliases (mostly for ForEach-Object, Where-Object, Select-Object) and some PSUseDeclaredVarsMoreThanAssignment warnings.
As part of this, it was also found that a test in ParameterBindingTests needed improvement because since the null comparison is now correct, the test failed initially and needed correcting.

@msftclas
Copy link
Copy Markdown

msftclas commented Jul 10, 2017

@bergmeister,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@bergmeister bergmeister mentioned this pull request Jul 10, 2017
Closed
@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Jul 11, 2017

@bergmeister Thanks for your conttribution!
Wait maintainer's reviews.

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Jul 11, 2017

@bergmeister Sorry, it seems @lzybkr meant that we should put each change kind in separate commit: %, ?, OutputType, $null and so on. I have no idea how this could be done now without a complete re-work.

@lzybkr Can we review as is?

@lzybkr
Copy link
Copy Markdown
Contributor

lzybkr commented Jul 11, 2017

If you do a soft reset, then git add -p, git will interactively ask you which diffs (not files, but individual diffs) to add. It's still time consuming, but it makes it much easier to manage big changes, e.g. if we need to use git bisect to narrow down a failure.

@bergmeister bergmeister mentioned this pull request Jul 14, 2017
Merged
@bergmeister
Copy link
Copy Markdown
Contributor Author

bergmeister commented Jul 14, 2017

OK. I have created #4261, which contains the separation of the commits as requested and will close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bergmeister @msftclas @iSazonov @lzybkr