Skip to content

Fixes of around 350 PSScriptAnalyzer warnings #4213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
bergmeister wants to merge 4 commits into from
Closed

Fixes of around 350 PSScriptAnalyzer warnings #4213

bergmeister wants to merge 4 commits into from

Conversation

@bergmeister
Copy link
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

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
Collaborator

iSazonov commented Jul 11, 2017

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

@iSazonov
Copy link
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
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
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