Skip to content

[ci] check PowerShell scripts with PSScriptAnalyzer (part 2)#6709

Merged
jameslamb merged 3 commits intomasterfrom
ci/psanalizer-2
Nov 1, 2024
Merged

[ci] check PowerShell scripts with PSScriptAnalyzer (part 2)#6709
jameslamb merged 3 commits intomasterfrom
ci/psanalizer-2

Conversation

@StrikerRUS
Copy link
Collaborator

Linking full original PR: #6700.
Continuation of #6704.

This PR fixes only all PSUseConsistentIndentation warnings, so it can be skimmed fast without attention.

pwsh -command "Install-Module -Name PSScriptAnalyzer -Scope CurrentUser -SkipPublisherCheck"
echo "Linting PowerShell code"
pwsh -file "./.ci/lint-powershell.ps1" || exit 0
pwsh -file "./.ci/lint-powershell.ps1" || :
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that after moving powershell linter upper all other ones, || exit 0 causes terminating the whole script earlier than running all linters.

Ref. for :

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right! Sorry 😅

|| true would work too. I'm indifferent, since this is temporary anyway

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the lint job logs (build link) and can confirm all the other linting is running, so looks like this works. Thanks very much for the Stack Overflow link, I hadn't seen || : before.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and I hadn't yet clicked that S/O link before suggesting || true. I see it says there:

The above suggestions to use 'true' will also work, but are inefficient as 'true' is an external program.

That's good to know! I have been using || true as a habit for a long tim.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been using || true as a habit for a long tim.

Me too. And hah, seems it's OK - just noticed the following comment under the S/O answer:

True is only an external program if you run /bin/true, as true on its own is a shell built-in of bash, just like echo and test, which also exists as external programs but are also built-ins in bash and if you don't give a full path, the built-ins will be used.

https://stackoverflow.com/questions/17830326/ignoring-specific-errors-in-a-shell-script/33427572#comment69729658_33427572

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wow!

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for doing the work of breaking these into smaller PRs! I'm glad to see this standardization of indentation in these scripts too 😁

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

This pull request has been automatically locked since there has not been any recent activity since it was closed.
To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants