Skip to content

PowerShellForGitHub: Improve and Standardise WhatIf/Confirm Processing #248

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

PowerShellForGitHub: Improve and Standardise WhatIf/Confirm Processing #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions GitHubAnalytics.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ function Group-GitHubIssue
$issues += Get-GitHubIssue -Uri 'https://github.com/powershell/xactivedirectory'
$issues | Group-GitHubIssue -Weeks 12 -DateType Closed
#>
[CmdletBinding(
SupportsShouldProcess,
DefaultParameterSetName='Weekly')]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[CmdletBinding(DefaultParameterSetName='Weekly')]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="DateType due to PowerShell/PSScriptAnalyzer#1472")]
param
(
Expand Down Expand Up @@ -165,10 +162,7 @@ function Group-GitHubPullRequest
$pullRequests += Get-GitHubPullRequest -Uri 'https://github.com/powershell/xactivedirectory'
$pullRequests | Group-GitHubPullRequest -Weeks 12 -DateType Closed
#>
[CmdletBinding(
SupportsShouldProcess,
DefaultParameterSetName='Weekly')]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[CmdletBinding(DefaultParameterSetName='Weekly')]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="DateType due to PowerShell/PSScriptAnalyzer#1472")]
param
(
Expand Down
54 changes: 27 additions & 27 deletions GitHubAssignees.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,8 @@ filter Get-GitHubAssignee

Lists the available assignees for issues from the microsoft\PowerShellForGitHub project.
#>
[CmdletBinding(
SupportsShouldProcess,
DefaultParameterSetName='Elements')]
[CmdletBinding(DefaultParameterSetName = 'Elements')]
[OutputType({$script:GitHubUserTypeName})]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[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.")]
param(
[Parameter(ParameterSetName='Elements')]
Expand Down Expand Up @@ -184,11 +181,8 @@ filter Test-GitHubAssignee
Checks if a user has permission to be assigned to an issue
from the microsoft\PowerShellForGitHub project.
#>
[CmdletBinding(
SupportsShouldProcess,
DefaultParameterSetName='Elements')]
[CmdletBinding(DefaultParameterSetName = 'Elements')]
[OutputType([bool])]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[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.")]
param(
[Parameter(ParameterSetName='Elements')]
Expand Down Expand Up @@ -341,7 +335,6 @@ function New-GitHubAssignee
SupportsShouldProcess,
DefaultParameterSetName='Elements')]
[OutputType({$script:GitHubIssueTypeName})]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[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.")]
param(
[Parameter(ParameterSetName='Elements')]
Expand Down Expand Up @@ -390,8 +383,6 @@ function New-GitHubAssignee

end
{
Write-InvocationLog

$elements = Resolve-RepositoryElements
$OwnerName = $elements.ownerName
$RepositoryName = $elements.repositoryName
Expand Down Expand Up @@ -419,6 +410,13 @@ function New-GitHubAssignee
'NoStatus' = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus -ConfigValueName DefaultNoStatus)
}

if (-not $PSCmdlet.ShouldProcess($Issue, "Add Assignee(s) $($userNames -join ',')"))
{
return
}

Write-InvocationLog

return (Invoke-GHRestMethod @params | Add-GitHubIssueAdditionalProperties)
}
}
Expand Down Expand Up @@ -575,8 +573,6 @@ function Remove-GitHubAssignee

end
{
Write-InvocationLog

$elements = Resolve-RepositoryElements
$OwnerName = $elements.ownerName
$RepositoryName = $elements.repositoryName
Expand All @@ -597,21 +593,25 @@ function Remove-GitHubAssignee
$ConfirmPreference = 'None'
}

if ($PSCmdlet.ShouldProcess($userNames -join ', ', "Remove assignee(s)"))
if (-not $PSCmdlet.ShouldProcess($Issue, "Remove assignee(s) $($userNames -join ', ')"))
{
$params = @{
'UriFragment' = "repos/$OwnerName/$RepositoryName/issues/$Issue/assignees"
'Body' = (ConvertTo-Json -InputObject $hashBody)
'Method' = 'Delete'
'Description' = "Removing assignees from issue $Issue for $RepositoryName"
'AccessToken' = $AccessToken
'AcceptHeader' = $script:symmetraAcceptHeader
'TelemetryEventName' = $MyInvocation.MyCommand.Name
'TelemetryProperties' = $telemetryProperties
'NoStatus' = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus -ConfigValueName DefaultNoStatus)
}

return (Invoke-GHRestMethod @params | Add-GitHubIssueAdditionalProperties)
return
}

Write-InvocationLog

$params = @{
'UriFragment' = "repos/$OwnerName/$RepositoryName/issues/$Issue/assignees"
'Body' = (ConvertTo-Json -InputObject $hashBody)
'Method' = 'Delete'
'Description' = "Removing assignees from issue $Issue for $RepositoryName"
'AccessToken' = $AccessToken
'AcceptHeader' = $script:symmetraAcceptHeader
'TelemetryEventName' = $MyInvocation.MyCommand.Name
'TelemetryProperties' = $telemetryProperties
'NoStatus' = (Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus -ConfigValueName DefaultNoStatus)
}

return (Invoke-GHRestMethod @params | Add-GitHubIssueAdditionalProperties)
}
}
5 changes: 1 addition & 4 deletions GitHubBranches.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,8 @@ filter Get-GitHubRepositoryBranch
again. This tries to show some of the different types of objects you can pipe into this
function.
#>
[CmdletBinding(
SupportsShouldProcess,
DefaultParameterSetName='Elements')]
[CmdletBinding(DefaultParameterSetName = 'Elements')]
[OutputType({$script:GitHubBranchTypeName})]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[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.")]
[Alias('Get-GitHubBranch')]
param(
Expand Down
103 changes: 58 additions & 45 deletions GitHubConfiguration.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ function Initialize-GitHubConfiguration
.NOTES
Internal helper method. This is actually invoked at the END of this file.
#>
[CmdletBinding(SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[CmdletBinding()]
param()

$script:seenTokenWarningThisSession = $false
Expand Down Expand Up @@ -179,7 +178,6 @@ function Set-GitHubConfiguration
[CmdletBinding(
PositionalBinding = $false,
SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
param(
[ValidatePattern('^(?!https?:)(?!api\.)(?!www\.).*')]
[string] $ApiHostName,
Expand Down Expand Up @@ -246,6 +244,11 @@ function Set-GitHubConfiguration
}
}

if (-not $PSCmdlet.ShouldProcess('GitHubConfiguration',' Save'))
{
return
}

if (-not $SessionOnly)
{
Save-GitHubConfiguration -Configuration $persistedConfig -Path $script:configurationFilePath
Expand Down Expand Up @@ -332,8 +335,7 @@ function Save-GitHubConfiguration

Serializes $config as a JSON object to 'c:\foo\config.json'
#>
[CmdletBinding()]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[CmdletBinding(SupportsShouldProcess)]
param(
[Parameter(Mandatory)]
[PSCustomObject] $Configuration,
Expand All @@ -342,6 +344,11 @@ function Save-GitHubConfiguration
[string] $Path
)

if (-not $PSCmdlet.ShouldProcess('GitHub Configuration','Save'))
{
return
}

$null = New-Item -Path $Path -Force
ConvertTo-Json -InputObject $Configuration |
Set-Content -Path $Path -Force -ErrorAction SilentlyContinue -ErrorVariable ev
Expand Down Expand Up @@ -507,12 +514,14 @@ function Reset-GitHubConfiguration

Set-TelemetryEvent -EventName Reset-GitHubConfiguration

if (-not $PSCmdlet.ShouldProcess('GitHub Configuration','Reset'))
{
return
}

if (-not $SessionOnly)
{
if ($PSCmdlet.ShouldProcess($script:configurationFilePath, "Delete configuration file"))
{
$null = Remove-Item -Path $script:configurationFilePath -Force -ErrorAction SilentlyContinue -ErrorVariable ev
}
$null = Remove-Item -Path $script:configurationFilePath -Force -ErrorAction SilentlyContinue -ErrorVariable ev

if (($null -ne $ev) -and ($ev.Count -gt 0) -and ($ev[0].FullyQualifiedErrorId -notlike 'PathNotFound*'))
{
Expand Down Expand Up @@ -555,8 +564,7 @@ function Read-GitHubConfiguration
Returns back an object with the deserialized object contained in the specified file,
if it exists and is valid.
#>
[CmdletBinding(SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[CmdletBinding()]
param(
[string] $Path
)
Expand Down Expand Up @@ -609,8 +617,7 @@ function Import-GitHubConfiguration
within a deserialized object from the content in $Path. The configuration object
is then returned.
#>
[CmdletBinding(SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[CmdletBinding()]
param(
[string] $Path
)
Expand Down Expand Up @@ -692,13 +699,17 @@ function Backup-GitHubConfiguration
Writes the user's current configuration file to c:\foo\config.json.
#>
[CmdletBinding(SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
param(
[string] $Path,

[switch] $Force
)

if (-not $PSCmdlet.ShouldProcess('GitHub Configuration', 'Backup'))
{
return
}

# Make sure that the path that we're going to be storing the file exists.
$null = New-Item -Path (Split-Path -Path $Path -Parent) -ItemType Directory -Force

Expand Down Expand Up @@ -734,14 +745,18 @@ function Restore-GitHubConfiguration
Makes the contents of c:\foo\config.json be the user's configuration for the module.
#>
[CmdletBinding(SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
param(
[ValidateScript({
if (Test-Path -Path $_ -PathType Leaf) { $true }
else { throw "$_ does not exist." }})]
[string] $Path
)

if (-not $PSCmdlet.ShouldProcess('GitHub Configuration', 'Restore'))
{
return
}

# Make sure that the path that we're going to be storing the file exists.
$null = New-Item -Path (Split-Path -Path $script:configurationFilePath -Parent) -ItemType Directory -Force

Expand Down Expand Up @@ -787,8 +802,7 @@ function Resolve-ParameterWithDefaultConfigurationValue
Checks to see if the NoStatus switch was provided by the user from the calling method. If
so, uses that value. otherwise uses the DefaultNoStatus value currently configured.
#>
[CmdletBinding(SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[CmdletBinding()]
param(
$BoundParameters = (Get-Variable -Name PSBoundParameters -Scope 1 -ValueOnly),

Expand Down Expand Up @@ -880,16 +894,13 @@ function Set-GitHubAuthentication
authentication, but keeps it in memory only for the duration of this PowerShell session..
#>
[CmdletBinding(SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUsePSCredentialType", "", Justification="The System.Management.Automation.Credential() attribute does not appear to work in PowerShell v4 which we need to support.")]
param(
[PSCredential] $Credential,

[switch] $SessionOnly
)

Write-InvocationLog

if (-not $PSBoundParameters.ContainsKey('Credential'))
{
$message = 'Please provide your GitHub API Token in the Password field. You can enter anything in the username field (it will be ignored).'
Expand All @@ -910,15 +921,20 @@ function Set-GitHubAuthentication
}

$script:accessTokenCredential = $Credential

if (-not $PSCmdlet.ShouldProcess('GitHub Authentication', 'Set'))
{
return
}

Write-InvocationLog

if (-not $SessionOnly)
{
if ($PSCmdlet.ShouldProcess("Store API token as a SecureString in a local file"))
{
$null = New-Item -Path $script:accessTokenFilePath -Force
$script:accessTokenCredential.Password |
ConvertFrom-SecureString |
Set-Content -Path $script:accessTokenFilePath -Force
}
$null = New-Item -Path $script:accessTokenFilePath -Force
$script:accessTokenCredential.Password |
ConvertFrom-SecureString |
Set-Content -Path $script:accessTokenFilePath -Force
}
}

Expand Down Expand Up @@ -953,28 +969,27 @@ function Clear-GitHubAuthentication
[switch] $SessionOnly
)

Write-InvocationLog

Set-TelemetryEvent -EventName Clear-GitHubAuthentication

if ($PSCmdlet.ShouldProcess("Clear memory cache"))
if (-not $PSCmdlet.ShouldProcess('GitHub Authentication', 'Clear'))
{
$script:accessTokenCredential = $null
return
}

Write-InvocationLog

$script:accessTokenCredential = $null

if (-not $SessionOnly)
{
if ($PSCmdlet.ShouldProcess("Clear file-based cache"))
{
Remove-Item -Path $script:accessTokenFilePath -Force -ErrorAction SilentlyContinue -ErrorVariable ev
Remove-Item -Path $script:accessTokenFilePath -Force -ErrorAction SilentlyContinue -ErrorVariable ev

if (($null -ne $ev) -and
($ev.Count -gt 0) -and
($ev[0].FullyQualifiedErrorId -notlike 'PathNotFound*'))
{
$message = "Experienced a problem trying to remove the file that persists the Access Token [$script:accessTokenFilePath]."
Write-Log -Message $message -Level Warning -Exception $ev[0]
}
if (($null -ne $ev) -and
($ev.Count -gt 0) -and
($ev[0].FullyQualifiedErrorId -notlike 'PathNotFound*'))
{
$message = "Experienced a problem trying to remove the file that persists the Access Token [$script:accessTokenFilePath]."
Write-Log -Message $message -Level Warning -Exception $ev[0]
}
}

Expand Down Expand Up @@ -1006,9 +1021,8 @@ function Get-AccessToken
.OUTPUTS
System.String
#>
[CmdletBinding(SupportsShouldProcess)]
[CmdletBinding()]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidGlobalVars", "", Justification="For back-compat with v0.1.0, this still supports the deprecated method of using a global variable for storing the Access Token.")]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[OutputType([String])]
param(
[string] $AccessToken
Expand Down Expand Up @@ -1085,8 +1099,7 @@ function Test-GitHubAuthenticationConfigured

Returns $true if the session is authenticated; $false otherwise
#>
[CmdletBinding(SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
[CmdletBinding()]
[OutputType([Boolean])]
param()

Expand Down
Loading