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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Nov 1, 2024

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.

@@ -100,7 +100,7 @@ fi
if [[ $TASK == "lint" ]]; then
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 😁

@jameslamb jameslamb merged commit 92aa07b into master Nov 1, 2024
48 checks passed
@jameslamb jameslamb deleted the ci/psanalizer-2 branch November 1, 2024 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants