[ci] check PowerShell scripts with PSScriptAnalyzer (part 1)#6704
[ci] check PowerShell scripts with PSScriptAnalyzer (part 1)#6704StrikerRUS merged 3 commits intomasterfrom
Conversation
StrikerRUS
requested review from
borchero,
guolinke,
jameslamb,
jmoralez and
shiyu1994
as code owners
jameslamb
left a comment
jameslamb
left a comment
There was a problem hiding this comment.
Thanks! The script changes look great to me... I agree with the choice of rules and and verbs. Thanks for the links in the description... I was not familiar with this concept of "approved verbs" in powershell before.
I just left one small suggestion on test.sh. Marking this "Approve", so you can merge without needing a new review from me if you agree with that suggestion.
.ci/test.sh
Outdated
| source activate "${CONDA_ENV}" | ||
| pwsh -command "Install-Module -Name PSScriptAnalyzer -Scope CurrentUser -SkipPublisherCheck" | ||
| echo "Linting PowerShell code" | ||
| pwsh -file "./.ci/lint-powershell.ps1" || exit 0 |
There was a problem hiding this comment.
Because this doesn't require anything installed with conda (we're relying on the pwsh that comes pre-installed on the runner), I think we should move this up to before conda create. Two reasons:
- prevents the case where we wait for
conda installto complete (which can sometimes take a few minutes) and then find out that the job is going to fail because of issues found byPSScriptAnalyzer. - prevents where
source activate "${CONDA ENV}"modifies the environment in a way that causes issues forpwsh(e.g. changesPATH/LD_LIBRARY_PATHand causes an incompatible mix of libraries to be loaded)
There was a problem hiding this comment.
Good catch! Totally makes sense. Will push changes right now.
|
This pull request has been automatically locked since there has not been any recent activity since it was closed. |
2 participants
Linking full original PR: #6700.
This PR fixes all warnings about our own functions (
PSUseApprovedVerbs,PSUseShouldProcessForStateChangingFunctions,PSUseSingularNouns).Also this PR makes params in our own functions mandatory where needed.
List of available rules: https://github.com/PowerShell/PSScriptAnalyzer/blob/main/docs/Rules/README.md.
List of Approved Verbs for PowerShell Commands: https://learn.microsoft.com/en-us/powershell/scripting/developer/cmdlet/approved-verbs-for-windows-powershell-commands?view=powershell-7.4.