Refactor error handling, proper termination using ErrorActionPreference #2158
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ommands Added ErrorPreference to 73 out of 82 total functions in source/Public and source/Private directories. Functions with only single statements (like simple Invoke-SetupAction calls) were correctly skipped. - Modified 61 Public functions - Modified 12 Private functions - Skipped 9 single-statement functions that don't need ErrorPreference The ErrorPreference is set immediately after the param block in each function to ensure terminating errors are properly handled for functions with multiple command invocations. Co-authored-by: johlju <[email protected]>
Copilot
AI
changed the title
[WIP] Do for each command (./source/Public) and private function (./source/Private) in the project, but only if the function has more that one command invocation and or statement:
- Always set
Add $ErrorPreference = 'Stop' to functions with multiple statements/commands
Aug 30, 2025
$ErrorPreference to Stop at start of function
johlju
requested changes
Aug 30, 2025
…reference Co-authored-by: johlju <[email protected]>
Copilot
AI
changed the title
Add $ErrorPreference = 'Stop' to functions with multiple statements/commands
Fix incorrect variable name: change $ErrorPreference to $ErrorActionPreference in functions
Aug 30, 2025
johlju
requested changes
Aug 30, 2025
…ess blocks Co-authored-by: johlju <[email protected]>
Copilot
AI
changed the title
Fix incorrect variable name: change $ErrorPreference to $ErrorActionPreference in functions
Fix PowerShell pipeline error handling: move $ErrorActionPreference into begin blocks
Aug 30, 2025
johlju
requested changes
Aug 30, 2025
Co-authored-by: johlju <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2158 +/- ##
====================================
Coverage 94% 94%
====================================
Files 127 127
Lines 8543 8567 +24
====================================
+ Hits 8090 8114 +24
Misses 453 453
🚀 New features to boost your workflow:
|
johlju
requested changes
Aug 31, 2025
…functions Co-authored-by: johlju <[email protected]>
Copilot
AI
changed the title
Fix PowerShell pipeline error handling: move $ErrorActionPreference into begin blocks
Remove $ErrorActionPreference = 'Stop' from PowerShell functions to follow best practices
Aug 31, 2025
…rrorAction 'Stop' Co-authored-by: johlju <[email protected]>
Copilot
AI
changed the title
Remove $ErrorActionPreference = 'Stop' from PowerShell functions to follow best practices
Improve error handling in PowerShell functions by removing global $ErrorActionPreference and adding targeted error control
Aug 31, 2025
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughAdds scoped overrides of $ErrorActionPreference to 'Stop' around specific assertions and operations, restoring the original value afterward. Applies try/finally where needed. One function removes a previous forced 'Stop' in a ShouldProcess block. No public signatures changed; overall control flow otherwise unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Cmdlet as Target Cmdlet/Assertion
Caller->>Caller: Save $originalErrorActionPreference
Caller->>Caller: Set $ErrorActionPreference = 'Stop'
rect rgba(220,235,255,0.5)
note over Caller,Cmdlet: Scoped strict error handling
Caller->>Cmdlet: Invoke -ErrorAction 'Stop'
end
alt Success
Caller->>Caller: Restore $ErrorActionPreference
else Error
note over Caller: finally ensures restoration
Caller->>Caller: Restore $ErrorActionPreference
end
sequenceDiagram
autonumber
participant Caller
participant SqlLogin as Remove-SqlDscLogin: Drop()
Caller->>SqlLogin: ShouldProcess? If yes, call Drop()
note over Caller,SqlLogin: No forced 'Stop' inside block
alt Ambient preference terminates
SqlLogin-->>Caller: Throw (terminating)
Caller-->Caller: catch if applicable
else Non-terminating
SqlLogin-->>Caller: Non-terminating error or warning
Caller-->Caller: finally restores original preference
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Join our Discord community for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
johlju
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
johlju
changed the title
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
source/Private/Invoke-ReportServerSetupAction.ps1 (1)
204-210: DRY the scoped error-preference pattern (optional)Consider a private helper (e.g., Invoke-WithErrorActionStop { scriptblock }) to reduce repetition across files.
source/Public/Add-SqlDscTraceFlag.ps1 (1)
137-137: Parameter name casing nitUse consistent casing for splatted parameter:
TraceFlaginstead ofTraceFLagfor readability and to match the declared parameter.- $setSqlDscTraceFlagParameters.TraceFLag = $desiredTraceFlags + $setSqlDscTraceFlagParameters.TraceFlag = $desiredTraceFlagssource/Public/Remove-SqlDscTraceFlag.ps1 (1)
146-156: Good use of try/finally; tiny naming nit on TraceFlag parameterPattern is correct around Set-SqlDscTraceFlag. Minor readability nit: use exact casing TraceFlag when setting the splat to avoid confusion during reviews and future refactors.
Example adjustment outside the changed hunk:
$setSqlDscTraceFlagParameters.TraceFlag = $desiredTraceFlags
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
source/Private/Get-FileVersionInformation.ps1(1 hunks)source/Private/Invoke-ReportServerSetupAction.ps1(1 hunks)source/Private/Invoke-SetupAction.ps1(2 hunks)source/Public/Add-SqlDscTraceFlag.ps1(2 hunks)source/Public/Get-SqlDscStartupParameter.ps1(1 hunks)source/Public/Import-SqlDscPreferredModule.ps1(3 hunks)source/Public/Remove-SqlDscLogin.ps1(0 hunks)source/Public/Remove-SqlDscTraceFlag.ps1(2 hunks)source/Public/Set-SqlDscStartupParameter.ps1(1 hunks)source/suffix.ps1(1 hunks)
💤 Files with no reviewable changes (1)
- source/Public/Remove-SqlDscLogin.ps1
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run in PowerShell, from repository root
- Build before running tests:
.\build.ps1 -Tasks build- Always run tests in new PowerShell session:
Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow guidelines over existing code patterns
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Public/Set-SqlDscStartupParameter.ps1source/Public/Get-SqlDscStartupParameter.ps1source/Public/Add-SqlDscTraceFlag.ps1source/Public/Remove-SqlDscTraceFlag.ps1source/suffix.ps1source/Public/Import-SqlDscPreferredModule.ps1source/Private/Invoke-SetupAction.ps1source/Private/Get-FileVersionInformation.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit configuration file
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: Lis...
Files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Public/Set-SqlDscStartupParameter.ps1source/Public/Get-SqlDscStartupParameter.ps1source/Public/Add-SqlDscTraceFlag.ps1source/Public/Remove-SqlDscTraceFlag.ps1source/suffix.ps1source/Public/Import-SqlDscPreferredModule.ps1source/Private/Invoke-SetupAction.ps1source/Private/Get-FileVersionInformation.ps1
source/**/*.ps1
⚙️ CodeRabbit configuration file
source/**/*.ps1: # Localization GuidelinesRequirements
- Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
- Use localized string keys, not hardcoded strings
- Assume
$script:localizedDatais availableString Files
- Commands/functions:
source/en-US/{MyModuleName}.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming Patterns
- Format:
Verb_FunctionName_Action(underscore separators), e.g.Get_Database_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Public/Set-SqlDscStartupParameter.ps1source/Public/Get-SqlDscStartupParameter.ps1source/Public/Add-SqlDscTraceFlag.ps1source/Public/Remove-SqlDscTraceFlag.ps1source/suffix.ps1source/Public/Import-SqlDscPreferredModule.ps1source/Private/Invoke-SetupAction.ps1source/Private/Get-FileVersionInformation.ps1
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Use -ErrorAction Stop on commands in integration tests so failures surface immediately
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Avoid empty catch blocks; prefer -ErrorAction SilentlyContinue
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Empty parameter block should be 'param ()'; otherwise, opening and closing parentheses on their own lines
Applied to files:
source/Public/Set-SqlDscStartupParameter.ps1source/Public/Add-SqlDscTraceFlag.ps1source/Private/Get-FileVersionInformation.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Implement the Force parameter pattern: if ($Force.IsPresent -and -not $Confirm) { $ConfirmPreference = 'None' }
Applied to files:
source/Public/Set-SqlDscStartupParameter.ps1source/Public/Import-SqlDscPreferredModule.ps1
📚 Learning: 2025-08-18T10:44:38.990Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: source/Public/New-SqlDscLogin.ps1:254-257
Timestamp: 2025-08-18T10:44:38.990Z
Learning: When implementing Force and Confirm parameter interaction in PowerShell functions with SupportsShouldProcess, use the pattern `if ($Force.IsPresent -and -not $Confirm)` to suppress confirmations. This ensures that explicit `-Confirm:$true` is respected even when `-Force` is present, preserving user intent when they explicitly request confirmation alongside Force.
Applied to files:
source/Public/Set-SqlDscStartupParameter.ps1source/Public/Import-SqlDscPreferredModule.ps1
📚 Learning: 2025-08-17T10:03:59.993Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:83-86
Timestamp: 2025-08-17T10:03:59.993Z
Learning: In PowerShell functions with SupportsShouldProcess, the $Confirm automatic variable defaults to $false when no -Confirm parameter is passed, and only becomes $true when -Confirm or -Confirm:$true is explicitly passed. The logic `if ($Force.IsPresent -and -not $Confirm)` correctly handles both cases where no -Confirm is passed and where -Confirm:$false is explicitly passed.
Applied to files:
source/Public/Set-SqlDscStartupParameter.ps1source/Public/Import-SqlDscPreferredModule.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Include a Force parameter for functions that use $PSCmdlet.ShouldContinue or $PSCmdlet.ShouldProcess
Applied to files:
source/Public/Set-SqlDscStartupParameter.ps1source/Public/Get-SqlDscStartupParameter.ps1source/Public/Import-SqlDscPreferredModule.ps1
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : When calling commands in integration tests, pass -Force where applicable to avoid prompts
Applied to files:
source/Public/Set-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Use -ErrorAction Stop on commands in integration tests so failures surface immediately
Applied to files:
source/Public/Set-SqlDscStartupParameter.ps1source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Avoid empty catch blocks; prefer -ErrorAction SilentlyContinue
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1source/suffix.ps1source/Public/Import-SqlDscPreferredModule.ps1source/Private/Get-FileVersionInformation.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : For state-changing functions, use SupportsShouldProcess; place ShouldProcess check immediately before each state change; $PSCmdlet.ShouldProcess must follow the required pattern
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:25:09.959Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:25:09.959Z
Learning: Applies to source/Public/*.ps1 : Separate reusable logic into private functions (keep public commands thin)
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Do not use Should -Not -Throw; invoke commands directly
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Avoid aliases; use full command names
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Avoid Write-Output; use return instead
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : For public commands, never use InModuleScope (except when retrieving localized strings)
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Use $PSCmdlet.ThrowTerminatingError() for terminating errors (except for classes) with relevant error category
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:21:35.582Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-08-29T17:21:35.582Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Use try/catch for error handling in resource methods
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1source/suffix.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-25T10:07:22.349Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-08-25T10:07:22.349Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; handle errors via try/catch
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1source/suffix.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Integration/**/*.ps1 : If an integration test requires a SQL Server Database Engine, start the Windows service in BeforeAll and stop it in AfterAll
Applied to files:
source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-17T10:48:15.384Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/suffix.ps1:24-24
Timestamp: 2025-08-17T10:48:15.384Z
Learning: In source/suffix.ps1, the Write-Verbose message in the catch block for Import-SqlDscPreferredModule does not need localization because the exception message from Import-SqlDscPreferredModule is already localized by that command, making it an edge case exception to the localization guidelines.
Applied to files:
source/suffix.ps1source/Public/Import-SqlDscPreferredModule.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Avoid global variables (exception: $global:DSCMachineStatus)
Applied to files:
source/suffix.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : .NOTES: include only critical info (constraints, side effects, security, version compatibility, breaking behavior) up to 2 short sentences
Applied to files:
source/suffix.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (6)
source/Public/Add-SqlDscTraceFlag.ps1 (1)
139-149: Scoped EAP handling with try/finally looks goodThis block correctly restores state regardless of success/failure.
source/Public/Remove-SqlDscTraceFlag.ps1 (1)
109-119: Scoped $ErrorActionPreference restore looks goodSaving/restoring around Get-SqlDscTraceFlag with try/finally prevents leaking state.
source/Private/Invoke-SetupAction.ps1 (1)
1418-1428: LGTM: validation wrapped with guaranteed restorationThe Assert-SetupActionProperties block correctly uses try/finally to restore $ErrorActionPreference.
source/Public/Import-SqlDscPreferredModule.ps1 (2)
75-96: Correct scoping and restoration around Get-SqlDscPreferredModuleUsing try/catch/finally ensures localized error and guaranteed restoration of $ErrorActionPreference.
147-157: LGTM: Import-Module guarded with try/finallyTemporary 'Stop' with restoration is consistent and safe.
source/suffix.ps1 (1)
16-26: LGTM: Scoped override prevents global side effectsImport-SqlDscPreferredModule runs under temporary 'Stop' and restores reliably; aligns with prior guidance about this file’s catch/verbose behavior.
Member
|
@copilot update changlog according to instructions |
Co-authored-by: johlju <[email protected]>
Contributor
Author
johlju
approved these changes
Aug 31, 2025
Member
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 11 of 73 files at r5, 55 of 65 files at r7, 3 of 3 files at r8, 9 of 9 files at r10, all commit messages.
Reviewable status: all files reviewed, 16 unresolved discussions (waiting on @copilot)
source/Public/Get-SqlDscStartupParameter.ps1 line 74 at r10 (raw file):
Assert-ElevatedUser -ErrorAction 'Stop' $ErrorActionPreference = $originalErrorActionPreference
This line should probably move below Assert-ManageServiceType.
johlju
reviewed
Aug 31, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.
This change is