Skip to content
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.

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

Already on GitHub? Sign in to your account

many pslint fixes #55862

Open
wants to merge 12 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@ShachafGoldstein
Copy link
Contributor

commented Apr 28, 2019

SUMMARY

Handles:

  • PSAvoidTrailingWhitespace
  • PSAvoidGlobalVars
  • PSAvoidAssignmentToAutomaticVariable
  • PSAvoidUsingCmdletAliases
  • PSAvoidUsingWriteHost
  • PSUseDeclaredVarsMoreThanAssignments
  • PSUsePSCredentialType
  • PSAvoidUsingPositionalParameters
  • PSAvoidUsingEmptyCatchBlock
  • PSAvoidUsingWMICmdlet

Original errors file

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

windows pslint errors

ADDITIONAL INFORMATION

More to come 馃槃

+label: windows

@ansibot

This comment was marked as resolved.

Copy link
Contributor

commented Apr 28, 2019

@ShachafGoldstein This PR was evaluated as a potentially problematic PR for the following reasons:

  • More than 50 changed files.

Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: #ansible-devel on irc.freenode.net

click here for bot help

@ansibot

This comment was marked as resolved.

Copy link
Contributor

commented Apr 30, 2019

@ShachafGoldstein this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot

This comment was marked as resolved.

Copy link
Contributor

commented Apr 30, 2019

@ShachafGoldstein this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ShachafGoldstein ShachafGoldstein force-pushed the ShachafGoldstein:PSLintFixes branch from b5c8a55 to 55f8f4b Apr 30, 2019

@ansibot ansibot removed the merge_commit label Apr 30, 2019

@ShachafGoldstein ShachafGoldstein force-pushed the ShachafGoldstein:PSLintFixes branch from 55f8f4b to 7867de0 Apr 30, 2019

@ShachafGoldstein ShachafGoldstein changed the title WIP: many pslint fixes many pslint fixes Apr 30, 2019

@ansibot

This comment was marked as resolved.

Copy link
Contributor

commented May 6, 2019

@ShachafGoldstein This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

ShachafGoldstein added some commits Apr 28, 2019

Handles:
PSAvoidTrailingWhitespace
PSAvoidGlobalVars
PSAvoidAssignmentToAutomaticVariable
PSAvoidUsingCmdletAliases
PSAvoidUsingWriteHost
PSUseDeclaredVarsMoreThanAssignments
PSUsePSCredentialType
PSAvoidUsingPositionalParameters
PSAvoidUsingEmptyCatchBlock
PSAvoidUsingWMICmdlet

Replaced Write-Host with Write-Output
Added smart reboot check for win_domain feature installation
Modify the Creation of the pagefileto fit to CIM
Changelog fragment addition
Ignore.txt without fixes
Revert "Test"
This reverts commit 35c5c06.
Revert "run test again"
This reverts commit 80eaf07.

@ShachafGoldstein ShachafGoldstein force-pushed the ShachafGoldstein:PSLintFixes branch from 309105f to f0fe430 May 7, 2019

@dagwieers
Copy link
Member

left a comment

I agree with 99% of the changes, however some of the changes modify the module's behaviour, or do not seem related to PSLint only. So I think we need to move those changes out of this PR to get this accepted.

@ansibot ansibot added needs_revision and removed core_review labels May 13, 2019

@ShachafGoldstein

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

  • change | Out-Null to > $null
  • Remove null assignments
lib/ansible/modules/windows/win_region.ps1 PSAvoidUsingEmptyCatchBlock # Keep
lib/ansible/modules/windows/win_uri.ps1 PSAvoidUsingEmptyCatchBlock # Keep
lib/ansible/modules/windows/win_domain.ps1 PSUseDeclaredVarsMoreThanAssignments
lib/ansible/modules/windows/win_domain_controller.ps1 PSUseDeclaredVarsMoreThanAssignments

This comment has been minimized.

Copy link
@mattclay

mattclay May 14, 2019

Member

This would be much easier to review if these entries remained on their original lines, since we want to make sure new entries are not being added.

This comment has been minimized.

Copy link
@ShachafGoldstein

ShachafGoldstein May 14, 2019

Author Contributor

I moved them to the end to have a better order in the file for future work

@ShachafGoldstein

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

When this one will be done, should i squash the commits to a single one again or leave it as it is?

@mattclay

This comment has been minimized.

Copy link
Member

commented May 14, 2019

We'll squash as part of the merge, so leave it as-is to simplify reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.