PowerShellForGitHub: Improve and Standardise WhatIf/Confirm Processing

[X-Guardian]

Description

This PR improves and standardises the WhatIf/Confirm ('ShouldProcess') processing across all the functions.

Summary of changes

• Moved the $PSCmdlet.ShouldProcess check out of Invoke-GHRestMethod and added it to the calling functions with the correct operation and target.
• Added WhatIf:$false and Confirm:$false to the Out-File Cmdlet call in the Write-Log function in the Helpers module. This will prevent the Write-Log function displaying WhatIf and Confirm prompts.
• Removed ShouldProcess from non-state changing functions.
• Modified the current ShouldProcess conditions to use an early return.
• Modified the Set-GitHubIssueLabel function to remove ConfirmImpact='High' and set ConfirmPreference to 'Low' if no labels have been specified instead.
• Modified the Update-GitHubRepository function to remove ConfirmImpact='High' and set ConfirmPreference to 'Low' if the repo is being renamed instead.

Issues Fixed

• Fixes PowerShellForGitHub: Suggested WhatIf/Confirm Processing Improvements #225

References

N/A

Checklist

• You actually ran the code that you just wrote, especially if you did just "one last quick change".
• Comment-based help added/updated, including examples.
• Static analysis is reporting back clean.
• New/changed code adheres to our coding guidelines.
• New/changed code continues to support the pipeline.
• Changes to the manifest file follow the manifest guidance.
• Unit tests were added/updated and are all passing. See testing guidelines. This includes making sure that all pipeline input variations have been covered.
• Relevant usage examples have been added/updated in USAGE.md.
• If desired, ensure your name is added to our Contributors list

[HowardWolosky]

A few lingering questions/nits, and then there's the need to handle the merge conflict in GitHubCore.ps1 and Telemetry.ps1 introduced by the recent change that removed Status.

[HowardWolosky]

Also -- please let me know in what order you'd prefer your changes to go in...if you're wanting this one to go in before your other changes or after.

[HowardWolosky]

After looking through this one more time, I think there is a flaw in the pattern of putting Write-InvocationLog after the $PSCmdlet.ShouldProcess() check...there are a number of functions that do validation work and will write error messages to the console/log file in the event that there was a problem. Those lines will all print without an logged invocation entry, which could both confusing as well as detrimental (no way to review the passed-in parameters).

I think that the $PSCmdlet.ShouldProcess() checks should remain where they are, but that the Write-InvocationLog entries should go back to their original spots.

[X-Guardian]

@HowardWolosky, this is ready for the next review.

[HowardWolosky]

Thanks for your patience here and all the hard work. Ready to get this merged in now.

[HowardWolosky]

/azp run PowerShellForGitHub-CI

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[HowardWolosky]

This change is causing the tests to fail right now, because we still have the check at line 157 that calls $PSCmdlet.ShouldProcess():

Invoke-SendTelemetryEvent' calls ShouldProcess/ShouldContinue but does not have the ShouldProcess attribute.

Stack trace
at line: 157 in /home/vsts/work/1/s/Telemetry.ps1
 ShouldProcess
 Severity: Warning

[X-Guardian]

Removed ShouldProcessBlock

[HowardWolosky]

/azp run PowerShellForGitHub-CI

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[HowardWolosky]

There are 7 UT failures with this change (all 7 are consistently failing across all 4 targets, 2 additionals are one-off issues I believe).

All 7 are in the GitHubBranches.tests.ps1, most likely around usage of -WhatIf in Get-* functions...

Expected an exception, with message 'Origin branch Missing-Branch not found' to be thrown, but the message was 'A parameter cannot be found that matches parameter name 'Whatif'.'. from /home/vsts/work/1/s/GitHubBranches.ps1:300 char:53
at <ScriptBlock>, /home/vsts/work/1/s/Tests/GitHubBranches.tests.ps1: line 221
at <ScriptBlock>, /home/vsts/work/1/s/Tests/GitHubBranches.tests.ps1: line 235

[X-Guardian]

@HowardWolosky, I've remove the redundant WhatIf/Confirm parameters. Can you re-run the CI?

[HowardWolosky]

/azp run PowerShellForGitHub-CI

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[HowardWolosky]

Nice to get this in. Thanks so much for spearheading this effort, @X-Guardian!