Skip to content

Update powershell.instructions.md #1125

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

Draft
time-by-waves wants to merge 3 commits into
base: staged
Choose a base branch
from
+88 −86
Draft

Update powershell.instructions.md #1125

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ada52ce
Update powershell.instructions.md
time-by-waves Mar 22, 2026
9313106
Update instructions/powershell.instructions.md
time-by-waves Mar 23, 2026
81d37e0
Update instructions/powershell.instructions.md
time-by-waves Mar 23, 2026
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
174 changes: 88 additions & 86 deletions instructions/powershell.instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ safe, and maintainable scripts. It aligns with Microsoft’s PowerShell cmdlet d

- **Alias Avoidance:**
- Use full cmdlet names
- Avoid using aliases in scripts (e.g., use Get-ChildItem instead of gci)
- Avoid using aliases in scripts (e.g., use `Get-ChildItem` instead of `gci`)
- Document any custom aliases
- Use full parameter names

### Example
### Example - Naming Conventions

```powershell
function Get-UserProfile {
Expand All @@ -49,6 +49,9 @@ function Get-UserProfile {
)

process {
$outputString = "Searching for: '$($Username)'"
Write-Verbose -Message $outputString
Write-Verbose -Message "Profile type: $ProfileType"
# Logic here
}
}
Expand All @@ -75,12 +78,14 @@ function Get-UserProfile {
- Enable tab completion where possible

- **Switch Parameters:**
- Use [switch] for boolean flags
- Avoid $true/$false parameters
- Default to $false when omitted
- Use clear action names
- **ALWAYS** use `[switch]` for boolean flags, never `[bool]`
- **NEVER** use `[bool]$Parameter` or assign default values
- Switch parameters default to `$false` when omitted
- Use clear, action-oriented names
- Test presence with `.IsPresent`
- Using `$true`/`$false` in parameter attributes (e.g., `Mandatory = $true`) is acceptable

### Example
### Example - Parameter Design

```powershell
function Set-ResourceConfiguration {
Expand All @@ -93,16 +98,24 @@ function Set-ResourceConfiguration {
[ValidateSet('Dev', 'Test', 'Prod')]
[string]$Environment = 'Dev',

# ✔️ CORRECT: Use `[switch]` with no default value
[Parameter()]
[switch]$Force,

# ❌ WRONG: Shows incorrect default assignment, however this is correct syntax (requires `[switch]` cast).
[Parameter()]
[switch]$Quiet = [switch]$true,

[Parameter()]
[ValidateNotNullOrEmpty()]
[string[]]$Tags
)

process {
# Logic here
# Use .IsPresent to check switch state
if ($Quiet.IsPresent) {
Write-Verbose "Quiet mode enabled"
}
Comment on lines +105 to +118
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

The "WRONG" switch example assigns a default of [switch]$true but the function later checks $Quiet.IsPresent. If -Quiet is omitted, .IsPresent will be $false even though the variable value is $true, so the example’s logic becomes inconsistent/misleading. Consider removing the defaulted switch example, moving it into a separate "avoid" snippet, or updating the check to match the demonstrated pattern.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@time-by-waves time-by-waves Mar 23, 2026

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

}
}
```
Expand Down Expand Up @@ -133,7 +146,7 @@ function Set-ResourceConfiguration {
- Return modified/created object with `-PassThru`
- Use verbose/warning for status updates

### Example
### Example - Pipeline and Output

```powershell
function Update-ResourceStatus {
Expand Down Expand Up @@ -163,7 +176,7 @@ function Update-ResourceStatus {
Name = $Name
Status = $Status
LastUpdated = $timestamp
UpdatedBy = $env:USERNAME
UpdatedBy = "$($env:USERNAME)"
}

# Only output if PassThru is specified
Expand Down Expand Up @@ -209,69 +222,32 @@ function Update-ResourceStatus {
- Support automation scenarios
- Document all required inputs

### Example
### Example - Error Handling and Safety

```powershell
function Remove-UserAccount {
[CmdletBinding(SupportsShouldProcess = $true, ConfirmImpact = 'High')]
function Remove-CacheFiles {
[CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'High')]
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

The "ShouldProcess Implementation" guidance above calls out [CmdletBinding(SupportsShouldProcess = $true)], but this example uses [CmdletBinding(SupportsShouldProcess, ...)]. Consider making the example match the documented pattern (or updating the guidance) to avoid mixed conventions in the same document.

Suggested change
[CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'High')]
[CmdletBinding(SupportsShouldProcess = $true, ConfirmImpact = 'High')]

Copilot uses AI. Check for mistakes.
param(
[Parameter(Mandatory, ValueFromPipeline)]
[ValidateNotNullOrEmpty()]
[string]$Username,

[Parameter()]
[switch]$Force
[Parameter(Mandatory)]
[string]$Path
)

begin {
Write-Verbose 'Starting user account removal process'
$ErrorActionPreference = 'Stop'
}

process {
try {
# Validation
if (-not (Test-UserExists -Username $Username)) {
$errorRecord = [System.Management.Automation.ErrorRecord]::new(
[System.Exception]::new("User account '$Username' not found"),
'UserNotFound',
[System.Management.Automation.ErrorCategory]::ObjectNotFound,
$Username
)
$PSCmdlet.WriteError($errorRecord)
return
}

# Confirmation
$shouldProcessMessage = "Remove user account '$Username'"
if ($Force -or $PSCmdlet.ShouldProcess($Username, $shouldProcessMessage)) {
Write-Verbose "Removing user account: $Username"

# Main operation
Remove-ADUser -Identity $Username -ErrorAction Stop
Write-Warning "User account '$Username' has been removed"
}
} catch [Microsoft.ActiveDirectory.Management.ADException] {
$errorRecord = [System.Management.Automation.ErrorRecord]::new(
$_.Exception,
'ActiveDirectoryError',
[System.Management.Automation.ErrorCategory]::NotSpecified,
$Username
)
$PSCmdlet.ThrowTerminatingError($errorRecord)
} catch {
$errorRecord = [System.Management.Automation.ErrorRecord]::new(
$_.Exception,
'UnexpectedError',
[System.Management.Automation.ErrorCategory]::NotSpecified,
$Username
)
$PSCmdlet.ThrowTerminatingError($errorRecord)
try {
$files = Get-ChildItem -Path $Path -Filter "*.cache" -ErrorAction Stop

# Demonstrates WhatIf support
if ($PSCmdlet.ShouldProcess($Path, 'Remove cache files')) {
$files | Remove-Item -Force
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

In the Remove-CacheFiles example, Remove-Item is invoked without -ErrorAction Stop. Many Remove-Item failures are non-terminating by default, so the catch block may never run and the example won’t actually demonstrate the intended error handling. Consider adding -ErrorAction Stop (or setting $ErrorActionPreference = 'Stop' within the try).

Suggested change
$files | Remove-Item -Force
$files | Remove-Item -Force -ErrorAction Stop

Copilot uses AI. Check for mistakes.
Write-Verbose "Removed $($files.Count) cache files from $Path"
}
}

end {
Write-Verbose 'User account removal process completed'
} catch {
$errorRecord = [System.Management.Automation.ErrorRecord]::new(
$_.Exception,
'RemovalFailed',
[System.Management.Automation.ErrorCategory]::NotSpecified,
$Path
)
$PSCmdlet.WriteError($errorRecord)
}
}
```
Expand Down Expand Up @@ -307,50 +283,76 @@ function Remove-UserAccount {
- Use `ForEach-Object` instead of `%`
- Use `Get-ChildItem` instead of `ls` or `dir`

---

## Full Example: End-to-End Cmdlet Pattern

```powershell
function New-Resource {
[CmdletBinding(SupportsShouldProcess = $true, ConfirmImpact = 'Medium')]
function Remove-UserAccount {
[CmdletBinding(SupportsShouldProcess = $true, ConfirmImpact = 'High')]
param(
[Parameter(Mandatory = $true,
ValueFromPipeline = $true,
ValueFromPipelineByPropertyName = $true)]
[Parameter(Mandatory, ValueFromPipeline)]
[ValidateNotNullOrEmpty()]
[string]$Name,
[string]$Username,

[Parameter()]
[ValidateSet('Development', 'Production')]
[string]$Environment = 'Development'
[switch]$Force
)

begin {
Write-Verbose 'Starting resource creation process'
Write-Verbose 'Starting user account removal process'
$ErrorActionPreference = $currentErrorActionValue
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

$currentErrorActionValue is used to set/restore $ErrorActionPreference, but it’s never assigned. As written, the function will overwrite $ErrorActionPreference with $null (or an undefined value) and won’t restore the original preference. Store the current value first (e.g., assign $currentErrorActionValue = $ErrorActionPreference before setting it to 'Stop') and then restore it in end.

Suggested change
$ErrorActionPreference = $currentErrorActionValue
$currentErrorActionValue = $ErrorActionPreference

Copilot uses AI. Check for mistakes.
$ErrorActionPreference = 'Stop'
}

process {
try {
if ($PSCmdlet.ShouldProcess($Name, 'Create new resource')) {
# Resource creation logic here
Write-Output ([PSCustomObject]@{
Name = $Name
Environment = $Environment
Created = Get-Date
})
# Validation
if (-not (Test-UserExists -Username $Username)) {
$errorRecord = [System.Management.Automation.ErrorRecord]::new(
[System.Exception]::new("User account '$Username' not found"),
'UserNotFound',
[System.Management.Automation.ErrorCategory]::ObjectNotFound,
$Username
)
$PSCmdlet.WriteError($errorRecord)
return
}

# ShouldProcess with Force parameter
if ($PSCmdlet.ShouldProcess($Username, "Remove user account")) {
# ShouldContinue for additional confirmation when -Force is not used
if ($Force -or $PSCmdlet.ShouldContinue("Are you sure you want to remove '$Username'?", "Confirm Removal")) {
Write-Verbose "Removing user account: $Username"
Comment on lines +322 to +326
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

The comment says "ShouldProcess with Force parameter", but -Force is not used to affect ShouldProcess/confirmation (it only skips ShouldContinue). Either update the comment to reflect the actual behavior or incorporate -Force into the confirmation logic so the example matches its explanation.

Copilot uses AI. Check for mistakes.

# Main operation
Remove-ADUser -Identity $Username -ErrorAction Stop
Write-Warning "User account '$Username' has been removed"
}
}
} catch [Microsoft.ActiveDirectory.Management.ADException] {
$errorRecord = [System.Management.Automation.ErrorRecord]::new(
$_.Exception,
'ActiveDirectoryError',
[System.Management.Automation.ErrorCategory]::NotSpecified,
$Username
)
$PSCmdlet.ThrowTerminatingError($errorRecord)
} catch {
$errorRecord = [System.Management.Automation.ErrorRecord]::new(
$_.Exception,
'ResourceCreationFailed',
'UnexpectedError',
[System.Management.Automation.ErrorCategory]::NotSpecified,
$Name
$Username
)
$PSCmdlet.ThrowTerminatingError($errorRecord)
}
}

end {
Write-Verbose 'Completed resource creation process'
Write-Verbose 'User account removal process completed'
# Set ErrorActionPreference back to the value it had
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

There’s trailing whitespace at the end of this comment line (after "had"), which can create noisy diffs later. Consider trimming it.

Suggested change
# Set ErrorActionPreference back to the value it had
# Set ErrorActionPreference back to the value it had

Copilot uses AI. Check for mistakes.
$ErrorActionPreference = $currentErrorActionValue
}
}
```