Skip to content

Fixes of around 350 PSScriptAnalyzer warnings - #4213

Closed
Christoph Bergmeister (bergmeister) wants to merge 4 commits into
PowerShell:masterfrom
bergmeister:PSScriptAnalyzerWarningFixesAfterRebasedCommits
Closed

Fixes of around 350 PSScriptAnalyzer warnings#4213
Christoph Bergmeister (bergmeister) wants to merge 4 commits into
PowerShell:masterfrom
bergmeister:PSScriptAnalyzerWarningFixesAfterRebasedCommits

Conversation

@bergmeister

@bergmeister Christoph Bergmeister (bergmeister) commented Jul 10, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Microsoft Contribution License Agreements (msftclas) commented Jul 10, 2017

Copy link
Copy Markdown

Christoph Bergmeister (@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

Closed
@iSazonov

Ilya (iSazonov) commented Jul 11, 2017

Copy link
Copy Markdown
Collaborator

Christoph Bergmeister (@bergmeister) Thanks for your conttribution!
Wait maintainer's reviews.

@iSazonov

Ilya (iSazonov) commented Jul 11, 2017

Copy link
Copy Markdown
Collaborator

Christoph Bergmeister (@bergmeister) Sorry, it seems Jason Shirk (@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.

Jason Shirk (@lzybkr) Can we review as is?

@lzybkr

Jason Shirk (lzybkr) commented Jul 11, 2017

Copy link
Copy Markdown
Contributor

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.

Merged
@bergmeister

Christoph Bergmeister (bergmeister) commented Jul 14, 2017

Copy link
Copy Markdown
Contributor Author

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