GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix errors in ShouldProcess rule document by masaru-iritani · Pull Request #1766 · PowerShell/PSScriptAnalyzer

Conversation

@masaru-iritani

@masaru-iritani Masaru Iritani (masaru-iritani) commented Jan 23, 2022

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Masaru Iritani (masaru-iritani) added 2 commits January 23, 2022 15:42
@masaru-iritani
Fix ShouldProcess examples
a4cc15c
@masaru-iritani
Fix Severity of ShouldProcess to Warning
7bddbc9
@bergmeister Christoph Bergmeister (bergmeister) self-assigned this Jan 23, 2022
@bergmeister
Christoph Bergmeister (bergmeister) self-requested a review January 23, 2022 20:40
bergmeister
Christoph Bergmeister (bergmeister) approved these changes Jan 28, 2022
View reviewed changes

@bergmeister Christoph Bergmeister (bergmeister) left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

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 🥳
Sean Wheeler (@sdwheeler) I am happy from a technical perspective, do you want to review the docs change as well before merging?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread docs/Rules/README.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bergmeister
Christoph Bergmeister (bergmeister) requested a review from Sean Wheeler (sdwheeler) January 28, 2022 20:15
sdwheeler
Sean Wheeler (sdwheeler) reviewed Jan 28, 2022
View reviewed changes
Comment thread docs/Rules/ShouldProcess.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

sdwheeler
Sean Wheeler (sdwheeler) suggested changes Jan 28, 2022
View reviewed changes

@sdwheeler Sean Wheeler (sdwheeler) left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@masaru-iritani
Add Get-Help
4eea026
@masaru-iritani
Masaru Iritani (masaru-iritani) requested a review from Sean Wheeler (sdwheeler) January 29, 2022 13:46
@masaru-iritani
Update links
e2d69dd
@masaru-iritani

Masaru Iritani (masaru-iritani) commented Feb 2, 2022

Copy link
Copy Markdown
Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bergmeister

Christoph Bergmeister (bergmeister) commented Mar 29, 2022

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sdwheeler
Copy example from Docs repo
36f32f0
sdwheeler
Sean Wheeler (sdwheeler) approved these changes Mar 29, 2022
View reviewed changes
bergmeister
Christoph Bergmeister (bergmeister) reviewed Mar 29, 2022
View reviewed changes
Comment thread docs/Rules/ShouldProcess.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

@sdwheeler
Update ShouldProcess.md
f2a247b
bergmeister
Christoph Bergmeister (bergmeister) approved these changes Mar 29, 2022
View reviewed changes
@bergmeister

Christoph Bergmeister (bergmeister) commented Mar 29, 2022

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sdwheeler
Merge branch 'master' into patch-1
4a5cb0c
bergmeister
Christoph Bergmeister (bergmeister) approved these changes Mar 29, 2022
View reviewed changes
@bergmeister
Christoph Bergmeister (bergmeister) enabled auto-merge (squash) March 29, 2022 22:56
@bergmeister Christoph Bergmeister (bergmeister) added the Area - Documentation label Mar 29, 2022
@JamesWTruher
James Truher (JamesWTruher) disabled auto-merge March 30, 2022 15:33
@JamesWTruher
James Truher (JamesWTruher) merged commit da59807 into PowerShell:master Mar 30, 2022
@masaru-iritani
Masaru Iritani (masaru-iritani) deleted the patch-1 branch April 2, 2022 06:51
@PowerShell PowerShell Team (PowerShell) deleted a comment from Toomani May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bergmeister Christoph Bergmeister (bergmeister) bergmeister approved these changes
+1 more reviewer
@sdwheeler Sean Wheeler (sdwheeler) sdwheeler approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@bergmeister Christoph Bergmeister (bergmeister)

Labels

Area - Documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@masaru-iritani @bergmeister @sdwheeler @JamesWTruher
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.