Skip to content

Commit 2f16de1

Browse files
X-GuardianX-Guardian
authored
PowerShellForGitHub: Improve and Standardise WhatIf/Confirm Processing (#254)
Improves and standardizes the WhatIf/Confirm ('ShouldProcess') processing across all the functions. * 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.
1 parent 981b85c commit 2f16de1

28 files changed

+676
-600
lines changed

Diff for: ‎GitHubAnalytics.ps1

+2-8
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,7 @@ function Group-GitHubIssue
3737
$issues += Get-GitHubIssue -Uri 'https://github.com/powershell/xactivedirectory'
3838
$issues | Group-GitHubIssue -Weeks 12 -DateType Closed
3939
#>
40-
[CmdletBinding(
41-
SupportsShouldProcess,
42-
DefaultParameterSetName='Weekly')]
43-
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
40+
[CmdletBinding(DefaultParameterSetName = 'Weekly')]
4441
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="DateType due to PowerShell/PSScriptAnalyzer#1472")]
4542
param
4643
(
@@ -165,10 +162,7 @@ function Group-GitHubPullRequest
165162
$pullRequests += Get-GitHubPullRequest -Uri 'https://github.com/powershell/xactivedirectory'
166163
$pullRequests | Group-GitHubPullRequest -Weeks 12 -DateType Closed
167164
#>
168-
[CmdletBinding(
169-
SupportsShouldProcess,
170-
DefaultParameterSetName='Weekly')]
171-
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
165+
[CmdletBinding(DefaultParameterSetName='Weekly')]
172166
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="DateType due to PowerShell/PSScriptAnalyzer#1472")]
173167
param
174168
(

Diff for: ‎GitHubAssignees.ps1

+23-23
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,8 @@ filter Get-GitHubAssignee
6464
6565
Lists the available assignees for issues from the microsoft\PowerShellForGitHub project.
6666
#>
67-
[CmdletBinding(
68-
SupportsShouldProcess,
69-
DefaultParameterSetName='Elements')]
67+
[CmdletBinding(DefaultParameterSetName = 'Elements')]
7068
[OutputType({$script:GitHubUserTypeName})]
71-
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
7269
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="One or more parameters (like NoStatus) are only referenced by helper methods which get access to it from the stack via Get-Variable -Scope 1.")]
7370
param(
7471
[Parameter(ParameterSetName='Elements')]
@@ -188,11 +185,8 @@ filter Test-GitHubAssignee
188185
Checks if a user has permission to be assigned to an issue
189186
from the microsoft\PowerShellForGitHub project.
190187
#>
191-
[CmdletBinding(
192-
SupportsShouldProcess,
193-
DefaultParameterSetName='Elements')]
188+
[CmdletBinding(DefaultParameterSetName = 'Elements')]
194189
[OutputType([bool])]
195-
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
196190
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="One or more parameters (like NoStatus) are only referenced by helper methods which get access to it from the stack via Get-Variable -Scope 1.")]
197191
param(
198192
[Parameter(ParameterSetName='Elements')]
@@ -348,7 +342,6 @@ function Add-GitHubAssignee
348342
DefaultParameterSetName='Elements')]
349343
[OutputType({$script:GitHubIssueTypeName})]
350344
[Alias('New-GitHubAssignee')] # Non-standard usage of the New verb, but done to avoid a breaking change post 0.14.0
351-
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
352345
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="One or more parameters (like NoStatus) are only referenced by helper methods which get access to it from the stack via Get-Variable -Scope 1.")]
353346
param(
354347
[Parameter(ParameterSetName='Elements')]
@@ -426,6 +419,11 @@ function Add-GitHubAssignee
426419
'NoStatus' = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus -ConfigValueName DefaultNoStatus)
427420
}
428421

422+
if (-not $PSCmdlet.ShouldProcess($Issue, "Add Assignee(s) $($userNames -join ',')"))
423+
{
424+
return
425+
}
426+
429427
return (Invoke-GHRestMethod @params | Add-GitHubIssueAdditionalProperties)
430428
}
431429
}
@@ -606,21 +604,23 @@ function Remove-GitHubAssignee
606604
$ConfirmPreference = 'None'
607605
}
608606

609-
if ($PSCmdlet.ShouldProcess($userNames -join ', ', "Remove assignee(s)"))
607+
if (-not $PSCmdlet.ShouldProcess($Issue, "Remove assignee(s) $($userNames -join ', ')"))
610608
{
611-
$params = @{
612-
'UriFragment' = "repos/$OwnerName/$RepositoryName/issues/$Issue/assignees"
613-
'Body' = (ConvertTo-Json -InputObject $hashBody)
614-
'Method' = 'Delete'
615-
'Description' = "Removing assignees from issue $Issue for $RepositoryName"
616-
'AccessToken' = $AccessToken
617-
'AcceptHeader' = $script:symmetraAcceptHeader
618-
'TelemetryEventName' = $MyInvocation.MyCommand.Name
619-
'TelemetryProperties' = $telemetryProperties
620-
'NoStatus' = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus -ConfigValueName DefaultNoStatus)
621-
}
622-
623-
return (Invoke-GHRestMethod @params | Add-GitHubIssueAdditionalProperties)
609+
return
624610
}
611+
612+
$params = @{
613+
'UriFragment' = "repos/$OwnerName/$RepositoryName/issues/$Issue/assignees"
614+
'Body' = (ConvertTo-Json -InputObject $hashBody)
615+
'Method' = 'Delete'
616+
'Description' = "Removing assignees from issue $Issue for $RepositoryName"
617+
'AccessToken' = $AccessToken
618+
'AcceptHeader' = $script:symmetraAcceptHeader
619+
'TelemetryEventName' = $MyInvocation.MyCommand.Name
620+
'TelemetryProperties' = $telemetryProperties
621+
'NoStatus' = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus -ConfigValueName DefaultNoStatus)
622+
}
623+
624+
return (Invoke-GHRestMethod @params | Add-GitHubIssueAdditionalProperties)
625625
}
626626
}

Diff for: ‎GitHubBranches.ps1

+55-55
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,8 @@ filter Get-GitHubRepositoryBranch
9797
again. This tries to show some of the different types of objects you can pipe into this
9898
function.
9999
#>
100-
[CmdletBinding(
101-
SupportsShouldProcess,
102-
DefaultParameterSetName='Elements')]
100+
[CmdletBinding(DefaultParameterSetName = 'Elements')]
103101
[OutputType({$script:GitHubBranchTypeName})]
104-
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
105102
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="One or more parameters (like NoStatus) are only referenced by helper methods which get access to it from the stack via Get-Variable -Scope 1.")]
106103
[Alias('Get-GitHubBranch')]
107104
param(
@@ -237,9 +234,6 @@ filter New-GitHubRepositoryBranch
237234
PositionalBinding = $false
238235
)]
239236
[OutputType({$script:GitHubBranchTypeName})]
240-
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSShouldProcess', '',
241-
Justification = 'Methods called within here make use of PSShouldProcess, and the switch is
242-
passed on to them inherently.')]
243237
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', '',
244238
Justification = 'One or more parameters (like NoStatus) are only referenced by helper
245239
methods which get access to it from the stack via Get-Variable -Scope 1.')]
@@ -291,8 +285,6 @@ filter New-GitHubRepositoryBranch
291285
OwnerName = $OwnerName
292286
RepositoryName = $RepositoryName
293287
BranchName = $BranchName
294-
Whatif = $false
295-
Confirm = $false
296288
}
297289
if ($PSBoundParameters.ContainsKey('AccessToken'))
298290
{
@@ -338,6 +330,11 @@ filter New-GitHubRepositoryBranch
338330
sha = $originBranch.commit.sha
339331
}
340332

333+
if (-not $PSCmdlet.ShouldProcess($BranchName, 'Create Repository Branch'))
334+
{
335+
return
336+
}
337+
341338
$params = @{
342339
'UriFragment' = $uriFragment
343340
'Body' = (ConvertTo-Json -InputObject $hashBody)
@@ -431,9 +428,6 @@ filter Remove-GitHubRepositoryBranch
431428
DefaultParameterSetName = 'Elements',
432429
PositionalBinding = $false,
433430
ConfirmImpact = 'High')]
434-
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "",
435-
Justification = "Methods called within here make use of PSShouldProcess, and the switch is
436-
passed on to them inherently.")]
437431
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "",
438432
Justification = "One or more parameters (like NoStatus) are only referenced by helper
439433
methods which get access to it from the stack via Get-Variable -Scope 1.")]
@@ -468,6 +462,8 @@ filter Remove-GitHubRepositoryBranch
468462
[switch] $NoStatus
469463
)
470464

465+
Write-InvocationLog
466+
471467
$elements = Resolve-RepositoryElements
472468
$OwnerName = $elements.ownerName
473469
$RepositoryName = $elements.repositoryName
@@ -484,23 +480,23 @@ filter Remove-GitHubRepositoryBranch
484480
$ConfirmPreference = 'None'
485481
}
486482

487-
if ($PSCmdlet.ShouldProcess($BranchName, "Remove Repository Branch"))
483+
if (-not $PSCmdlet.ShouldProcess($BranchName, "Remove Repository Branch"))
488484
{
489-
Write-InvocationLog
490-
491-
$params = @{
492-
'UriFragment' = $uriFragment
493-
'Method' = 'Delete'
494-
'Description' = "Deleting branch $BranchName from $RepositoryName"
495-
'AccessToken' = $AccessToken
496-
'TelemetryEventName' = $MyInvocation.MyCommand.Name
497-
'TelemetryProperties' = $telemetryProperties
498-
'NoStatus' = (Resolve-ParameterWithDefaultConfigurationValue `
499-
-Name NoStatus -ConfigValueName DefaultNoStatus)
500-
}
485+
return
486+
}
501487

502-
Invoke-GHRestMethod @params | Out-Null
488+
$params = @{
489+
'UriFragment' = $uriFragment
490+
'Method' = 'Delete'
491+
'Description' = "Deleting branch $BranchName from $RepositoryName"
492+
'AccessToken' = $AccessToken
493+
'TelemetryEventName' = $MyInvocation.MyCommand.Name
494+
'TelemetryProperties' = $telemetryProperties
495+
'NoStatus' = (Resolve-ParameterWithDefaultConfigurationValue `
496+
-Name NoStatus -ConfigValueName DefaultNoStatus)
503497
}
498+
499+
Invoke-GHRestMethod @params | Out-Null
504500
}
505501

506502
filter Get-GitHubRepositoryBranchProtectionRule
@@ -965,27 +961,29 @@ filter New-GitHubRepositoryBranchProtectionRule
965961
$hashBody['allow_deletions'] = $AllowDeletions.ToBool()
966962
}
967963

968-
if ($PSCmdlet.ShouldProcess(
964+
if (-not $PSCmdlet.ShouldProcess(
969965
"'$BranchName' branch of repository '$RepositoryName'",
970966
'Create GitHub Repository Branch Protection Rule'))
971967
{
972-
$jsonConversionDepth = 3
973-
974-
$params = @{
975-
UriFragment = "repos/$OwnerName/$RepositoryName/branches/$BranchName/protection"
976-
Body = (ConvertTo-Json -InputObject $hashBody -Depth $jsonConversionDepth)
977-
Description = "Setting $BranchName branch protection status for $RepositoryName"
978-
Method = 'Put'
979-
AcceptHeader = $script:lukeCageAcceptHeader
980-
AccessToken = $AccessToken
981-
TelemetryEventName = $MyInvocation.MyCommand.Name
982-
TelemetryProperties = $telemetryProperties
983-
NoStatus = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus `
984-
-ConfigValueName DefaultNoStatus)
985-
}
968+
return
969+
}
970+
971+
$jsonConversionDepth = 3
986972

987-
return (Invoke-GHRestMethod @params | Add-GitHubBranchProtectionRuleAdditionalProperties)
973+
$params = @{
974+
UriFragment = "repos/$OwnerName/$RepositoryName/branches/$BranchName/protection"
975+
Body = (ConvertTo-Json -InputObject $hashBody -Depth $jsonConversionDepth)
976+
Description = "Setting $BranchName branch protection status for $RepositoryName"
977+
Method = 'Put'
978+
AcceptHeader = $script:lukeCageAcceptHeader
979+
AccessToken = $AccessToken
980+
TelemetryEventName = $MyInvocation.MyCommand.Name
981+
TelemetryProperties = $telemetryProperties
982+
NoStatus = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus `
983+
-ConfigValueName DefaultNoStatus)
988984
}
985+
986+
return (Invoke-GHRestMethod @params | Add-GitHubBranchProtectionRuleAdditionalProperties)
989987
}
990988

991989
filter Remove-GitHubRepositoryBranchProtectionRule
@@ -1100,23 +1098,25 @@ filter Remove-GitHubRepositoryBranchProtectionRule
11001098
$ConfirmPreference = 'None'
11011099
}
11021100

1103-
if ($PSCmdlet.ShouldProcess("'$BranchName' branch of repository '$RepositoryName'",
1101+
if (-not $PSCmdlet.ShouldProcess("'$BranchName' branch of repository '$RepositoryName'",
11041102
'Remove GitHub Repository Branch Protection Rule'))
11051103
{
1106-
$params = @{
1107-
UriFragment = "repos/$OwnerName/$RepositoryName/branches/$BranchName/protection"
1108-
Description = "Removing $BranchName branch protection rule for $RepositoryName"
1109-
Method = 'Delete'
1110-
AcceptHeader = $script:lukeCageAcceptHeader
1111-
AccessToken = $AccessToken
1112-
TelemetryEventName = $MyInvocation.MyCommand.Name
1113-
TelemetryProperties = $telemetryProperties
1114-
NoStatus = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus `
1115-
-ConfigValueName DefaultNoStatus)
1116-
}
1104+
return
1105+
}
11171106

1118-
return Invoke-GHRestMethod @params | Out-Null
1107+
$params = @{
1108+
UriFragment = "repos/$OwnerName/$RepositoryName/branches/$BranchName/protection"
1109+
Description = "Removing $BranchName branch protection rule for $RepositoryName"
1110+
Method = 'Delete'
1111+
AcceptHeader = $script:lukeCageAcceptHeader
1112+
AccessToken = $AccessToken
1113+
TelemetryEventName = $MyInvocation.MyCommand.Name
1114+
TelemetryProperties = $telemetryProperties
1115+
NoStatus = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus `
1116+
-ConfigValueName DefaultNoStatus)
11191117
}
1118+
1119+
return Invoke-GHRestMethod @params | Out-Null
11201120
}
11211121

11221122
filter Add-GitHubBranchAdditionalProperties

0 commit comments

Comments
 (0)