Skip to content

Remove use of cmdlet aliases from .\test\powershell#8546

Merged
iSazonov merged 7 commits intoPowerShell:masterfrom
xtqqczze:PSAvoidUsingCmdletAliases
Dec 28, 2018
Merged

Remove use of cmdlet aliases from .\test\powershell#8546
iSazonov merged 7 commits intoPowerShell:masterfrom
xtqqczze:PSAvoidUsingCmdletAliases

Conversation

@xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Dec 27, 2018

PR Summary

  • Remove use of cmdlet aliases from .\test\powershell directory.

PR Checklist

Remove use of cmdlet aliases from .\test\powershell\* and suppress warnings from PSScriptAnalyzer if use of a cmdlet alias is necessary for tests.
Fix for failing test and remove accidentally committed files under .\test\tools\Modules\SelfSignedCertificate
@iSazonov
Copy link
Collaborator

@xtqqczze Thanks for your contribution!

We never used message suppressions and I would delete its. In the case we could fast merge. If you think that this could be useful please open new issue for discussion.

Remove PSAvoidUsingCmdletAliases message suppression.
@iSazonov
Copy link
Collaborator

@xtqqczze Sorry, I meant that comments is not needed too. We don't have strong CI checks for scripts. So such suppressions and related comments makes no sense.

@xtqqczze
Copy link
Contributor Author

@iSazonov When using vscode with the ms-vscode.powershell extension enabled, rule violations from PSScriptAnalyzer testing are listed as warnings in the 'Problems' pane, for example:

{
	"resource": "~/test/powershell/Modules/Microsoft.PowerShell.Utility/Set-Variable.Tests.ps1",
	"owner": "_generated_diagnostic_collection_name_#2",
	"code": "PSAvoidUsingCmdletAliases",
	"severity": 4,
	"message": "'sv' is an alias of 'Set-Variable'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.",
	"source": "PSScriptAnalyzer",
	"startLineNumber": 178,
	"startColumn": 2,
	"endLineNumber": 178,
	"endColumn": 4
}

Certain tests have comments explaining a rule should be ignored, because at the present time rules can only be suppressed on a file level for Pester tests. These comments help identify spurious warnings in the vscode 'Problems' pane and provide a placeholder for future implementation of rule suppression at a test level.

@iSazonov
Copy link
Collaborator

iSazonov commented Dec 28, 2018

@xtqqczze I see your point. These comments is obvious and don't add value but reduce readability. Your proposal will be useful after we (1) get implementation of local suppressions and (2) force CIs. Today I suggest split replacing aliases and suppressing messages. For second work you could open a tracking issue if you want.

@iSazonov iSazonov requested a review from SteveL-MSFT December 28, 2018 06:02
@iSazonov iSazonov merged commit 70de294 into PowerShell:master Dec 28, 2018
@iSazonov
Copy link
Collaborator

@xtqqczze Thanks for your contribution!

@iSazonov iSazonov mentioned this pull request Dec 29, 2018
11 tasks
@xtqqczze xtqqczze deleted the PSAvoidUsingCmdletAliases branch January 2, 2019 01:45
xtqqczze added a commit to xtqqczze/PowerShell-PowerShell that referenced this pull request Jan 2, 2019
@xtqqczze xtqqczze mentioned this pull request Jan 12, 2019
11 tasks
xtqqczze added a commit to xtqqczze/PowerShell-PowerShell that referenced this pull request Jan 14, 2019
* Avoid use of sleep alias (follow-up to PowerShell#8546)
* Specify default parameter name
* Shorten overly specific comments
iSazonov pushed a commit that referenced this pull request Jan 15, 2019
* Avoid use of sleep alias (follow-up to #8546)
* Specify default parameter name
* Shorten overly specific comments
@iSazonov iSazonov added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Jan 17, 2019
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

Comments