◐ Shell
clean mode source ↗

Fix errors in ShouldProcess rule document by masaru-iritani · Pull Request #1766 · PowerShell/PSScriptAnalyzer

masaru-iritani

Skip to content

Navigation Menu

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sign up

Appearance settings

Conversation

@masaru-iritani

PR Summary

  • Fix wrong variable names in examples.
  • Remove Write-Host from the correct example. It is duplicated with outputs of -WhatIf or -Confirm. It also violates "Avoid Using Write-Host" rule.
  • Correct the severity of ShouldProcess rule as mentioned in ShouldProcess.md. Get-ScriptAnalyzerRule -Name PSShouldProcess | % Severity also returns "Warning" with ScriptAnalyzer 1.20.0

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Change is not breaking
  • Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
  • Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
  • This PR is ready to merge and is not Work in Progress.
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
  • Update examples can run successfully on PowerShell 7.2.1.
  • Script Analyzer 1.20.0 detects a ShouldProcess violation in the updated wrong example. For some reason, Script Analyzer 1.20.0 doesn't detect any ShouldProcess violation in the updated wrong example.
  • Script Analyzer 1.20.0 detects no error in the updated correct example.

bergmeister

Choose a reason for hiding this comment

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

Awesome, thank you for the effort and very detailed description 🥳
@sdwheeler I am happy from a technical perspective, do you want to review the docs change as well before merging?

sdwheeler

sdwheeler

Choose a reason for hiding this comment

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

One minor change to link to about_* topics.

@masaru-iritani

One minor change to link to about_* topics.

Updated as suggested. Would you review it again and resolve the request? I cannot find any way to resolve this thread by myself.

@bergmeister

@sdwheeler Can you re-review please? You can contact @JamesWTruher to get it merged as the left-over check doesn't seem to go away when pulling in master.

sdwheeler

bergmeister

bergmeister

@bergmeister

Thanks @sdwheeler, can you resolve the merge conflict please and then we are good to merge 💪🏻

bergmeister

Labels