Fix style for terminating error in commands#2312
Conversation
…s for compatibility level and collation validation
…wTerminatingError
WalkthroughRefactors terminating-error handling across multiple public cmdlets to use $PSCmdlet.ThrowTerminatingError() with constructed ErrorRecords and error IDs; adds scoped ErrorActionPreference/ConfirmPreference adjustments, ShouldProcess message updates and localization key changes; updates tests and a test SMO enum. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Cmdlet as "Public Cmdlet"
participant Validator as "Validation / SMO"
participant Host as "PowerShell Host"
Caller->>Cmdlet: Invoke (params, -Force/-Confirm)
Cmdlet->>Cmdlet: adjust ConfirmPreference / scope $ErrorActionPreference
Cmdlet->>Validator: validate inputs / perform SMO operation
alt validation/operation fails
Validator-->>Cmdlet: exception
Cmdlet->>Host: $PSCmdlet.ThrowTerminatingError(ErrorRecord)
Note right of Host #f7fbff: Terminating error surfaced\n(ErrorRecord: ID, Category, Target)
Host->>Caller: terminating error (ErrorRecord)
else success
Validator-->>Cmdlet: result
Cmdlet-->>Caller: return result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
source/Public/New-SqlDscDatabase.ps1 (1)
151-158: Good switch to structured terminating errors; add a guard for future SQL versions.If $ServerObject.VersionMajor isn’t in $supportedCompatibilityLevels, the lookup returns $null and any value will be treated as invalid. Add a fallback to the highest known set (or skip validation) to avoid false negatives on future releases.
Example minimal change:
- if ($CompatibilityLevel -notin $supportedCompatibilityLevels.$($ServerObject.VersionMajor)) + $allowed = $supportedCompatibilityLevels[$ServerObject.VersionMajor] + if (-not $allowed) { $allowed = $supportedCompatibilityLevels[[int]($supportedCompatibilityLevels.Keys | Measure-Object -Maximum).Maximum] } + if ($CompatibilityLevel -notin $allowed)source/Public/Remove-SqlDscRole.ps1 (2)
97-104: Use ObjectNotFound category for missing role.The error fits ObjectNotFound better than InvalidOperation. Consider:
- [System.InvalidOperationException]::new($errorMessage), - 'RSDR0001', - [System.Management.Automation.ErrorCategory]::InvalidOperation, + [System.Management.Automation.ItemNotFoundException]::new($errorMessage), + 'RSDR0001', + [System.Management.Automation.ErrorCategory]::ObjectNotFound,
119-125: Built-in role guard: terminating error usage looks good.InvalidOperation with localized message and target object is appropriate here. LGTM. As a consistency tweak, consider adopting the Force confirmation pattern (
$ConfirmPreference = 'None') instead of bypassing ShouldProcess when-Forceis used.source/Public/Remove-SqlDscDatabase.ps1 (2)
170-201: Use SMO enum for UserAccess; keep the robust restore of ErrorActionPreference.Assigning
'Single'relies on implicit conversion; prefer the enum to avoid brittleness across SMO versions.- $DatabaseObject.UserAccess = 'Single' + $DatabaseObject.UserAccess = [Microsoft.SqlServer.Management.Smo.DatabaseUserAccess]::Single
205-214: Removal flow and verbose telemetry are fine; structure looks good.Try/restore pattern with a dedicated ErrorId is solid. Consider adopting the Force confirmation pattern (
if ($Force.IsPresent -and -not $Confirm) { $ConfirmPreference = 'None' }) for consistency with other commands.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CHANGELOG.md(1 hunks)source/Public/New-SqlDscDatabase.ps1(2 hunks)source/Public/Remove-SqlDscAgentAlert.ps1(2 hunks)source/Public/Remove-SqlDscDatabase.ps1(5 hunks)source/Public/Remove-SqlDscRole.ps1(3 hunks)source/Public/Set-SqlDscAgentAlert.ps1(2 hunks)source/Public/Set-SqlDscDatabase.ps1(4 hunks)tests/Unit/Public/New-SqlDscDatabase.Tests.ps1(1 hunks)tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1(1 hunks)tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1(4 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: All public commands, private functions, and classes must have unit tests
All public commands and class-based resources must have integration tests
Use Pester v5 syntax only
Test code only inside Describe blocks
Assertions only in It blocks
Never test verbose messages, debug messages, or parameter binding behavior
Pass all mandatory parameters to avoid prompts
Inside It blocks, assign unused return objects to $null (unless part of a pipeline)
Call the tested entity from within the It blocks
Keep results and assertions in the same It block
Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Avoid unnecessary remove/recreate cycles
One Describe block per file matching the tested entity name
Context descriptions start with 'When'
It descriptions start with 'Should' and must not contain 'when'
Mock variables must be prefixed with 'mock'
Public commands: never use InModuleScope (except for retrieving localized strings)
Private functions and class resources: always use InModuleScope
Each class method gets a separate Context block
Each scenario gets a separate Context block
Use nested Context blocks for complex scenarios
Perform mocking in BeforeAll (use BeforeEach only when required)
Place setup/teardown (BeforeAll/BeforeEach/AfterAll/AfterEach) close to usage
Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Never use Assert-MockCalled; use Should -Invoke instead
Do not use Should -Not -Throw; invoke commands directly
Never add an empty -MockWith block
Omit -MockWith when returning $null
Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Omit the -ModuleName parameter on Pester commands
Never use Mock inside an InModuleScope block
Define variables for -ForEach in a separate BeforeDiscovery near usage
Use -ForEach only on Context and It blocks
Keep variable scope close to the usage c...
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1
tests/Unit/Public/*.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
Public command unit tests must reside in tests/Unit/Public/{Name}.Tests.ps1
Place unit tests for public commands in tests/Unit/Public/{Name}.Tests.ps1
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place 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)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Write-Output; use return instead
Do not use ConvertTo-SecureString -AsPlainText in production code
Do not redefine reserved parameters (Verbose, Debug, etc.)
In...
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Set-SqlDscDatabase.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1source/Public/Set-SqlDscAgentAlert.ps1source/Public/Remove-SqlDscRole.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1source/Public/New-SqlDscDatabase.ps1source/Public/Remove-SqlDscAgentAlert.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Set-SqlDscDatabase.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1source/Public/Set-SqlDscAgentAlert.ps1source/Public/Remove-SqlDscRole.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1source/Public/New-SqlDscDatabase.ps1source/Public/Remove-SqlDscAgentAlert.ps1
tests/[Uu]nit/**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md)
tests/[Uu]nit/**/*.[Tt]ests.ps1: In unit tests, access localized strings using InModuleScope -ScriptBlock { $script:localizedData.Key }
When mocking files in tests, use the $TestDrive variable for file system operations
All public commands must include parameter set validation tests
Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Use the provided Parameter Set Validation test template to assert command ParameterSetName and full parameter list string; for multiple parameter sets, supply multiple hashtables via -ForEach
Use the Parameter Properties template to assert a parameter is mandatory via $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName']; $parameterInfo.Attributes.Mandatory | Should -BeTrue
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[Uu]nit/**/*.[Tt]ests.ps1: # Unit Tests Guidelines
- Test with localized strings: Use
InModuleScope -ScriptBlock { $script:localizedData.Key }- Mock files: Use
$TestDrivevariable (path to the test drive)- All public commands require parameter set validation tests
- After modifying classes, always run tests in new session (for changes to take effect)
Test Setup Requirements
Use this exact setup block before
Describe:[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName } AfterAll { $PSDefaultParameterValues.Remove('InModuleScope:ModuleName') $PSDefaultParameterValues.Remove('Mock:ModuleNam...
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Set-SqlDscDatabase.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1source/Public/Set-SqlDscAgentAlert.ps1source/Public/Remove-SqlDscRole.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1source/Public/New-SqlDscDatabase.ps1source/Public/Remove-SqlDscAgentAlert.ps1
tests/Unit/{Classes,Public,Private}/*.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Add unit tests for all commands, functions, and resources
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession: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 instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- 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
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Set-SqlDscDatabase.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1source/Public/Set-SqlDscAgentAlert.ps1source/Public/Remove-SqlDscRole.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1source/Public/New-SqlDscDatabase.ps1source/Public/Remove-SqlDscAgentAlert.ps1CHANGELOG.md
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # 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...
Files:
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Set-SqlDscDatabase.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1source/Public/Set-SqlDscAgentAlert.ps1source/Public/Remove-SqlDscRole.ps1tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1source/Public/New-SqlDscDatabase.ps1source/Public/Remove-SqlDscAgentAlert.ps1
source/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/**/*.ps1: Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning, and $PSCmdlet.ThrowTerminatingError() messages
Use localized string keys instead of hardcoded strings in script output/messages
Assume and use $script:localizedData for accessing localized strings
When emitting messages, reference $script:localizedData.KeyName and format with the -f operator (e.g., Write-Verbose -Message ($script:localizedData.KeyName -f $value1))
Files:
source/Public/Remove-SqlDscDatabase.ps1source/Public/Set-SqlDscDatabase.ps1source/Public/Set-SqlDscAgentAlert.ps1source/Public/Remove-SqlDscRole.ps1source/Public/New-SqlDscDatabase.ps1source/Public/Remove-SqlDscAgentAlert.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/Public/Remove-SqlDscDatabase.ps1source/Public/Set-SqlDscDatabase.ps1source/Public/Set-SqlDscAgentAlert.ps1source/Public/Remove-SqlDscRole.ps1source/Public/New-SqlDscDatabase.ps1source/Public/Remove-SqlDscAgentAlert.ps1
source/Public/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Place public commands in source/Public/{CommandName}.ps1
Files:
source/Public/Remove-SqlDscDatabase.ps1source/Public/Set-SqlDscDatabase.ps1source/Public/Set-SqlDscAgentAlert.ps1source/Public/Remove-SqlDscRole.ps1source/Public/New-SqlDscDatabase.ps1source/Public/Remove-SqlDscAgentAlert.ps1
**/*.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)
**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown documents
Use '1.' for all items in ordered lists (1/1/1 numbering style)
Disable MD013 for tables/code blocks exceeding 80 characters via an inline comment
Require empty lines before and after code blocks and headings (except before line 1)
Escape backslashes in file paths only, not inside code blocks
All fenced code blocks must specify a language identifier
Format parameter names as bold
Format values/literals as inline code
Format resource/module/product names as italic
Format commands, file names, and paths as inline code
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)
CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format
Describe notable changes briefly, with no more than 2 items per change type
Reference issues using the format issue #<issue_number>
No empty lines between list items in the same section
Skip adding an entry if the same change already exists in the Unreleased section
No duplicate sections or items in the Unreleased sectionAlways update the Unreleased section of CHANGELOG.md
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : For terminating errors use $PSCmdlet.ThrowTerminatingError() (not in classes); include relevant error category and in catch include exception with localized message
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Applied to files:
source/Public/Remove-SqlDscRole.ps1source/Public/New-SqlDscDatabase.ps1
📚 Learning: 2025-08-18T13:50:53.789Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Applied to files:
tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-09-12T13:20:14.850Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-09-12T13:20:14.850Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : For terminating errors, use New-*Exception cmdlets: New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException
Applied to files:
source/Public/New-SqlDscDatabase.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 (17)
source/Public/Set-SqlDscAgentAlert.ps1 (2)
145-152: Terminating error shape and category are spot on.Item-not-found path correctly uses ObjectNotFound with a specific error ID and localized message. LGTM.
221-228: Catch block preserves inner exception and uses a specific ErrorId — good.Structured ErrorRecord with InvalidOperation and target object looks correct.
source/Public/Remove-SqlDscRole.ps1 (1)
146-153: Failure on Drop(): preserves inner exception and uses a specific ErrorId.Structured error is correct and helpful. LGTM.
CHANGELOG.md (1)
198-215: Changelog entry formatting and scope look correct.Items are under “Changed”, reference issues properly, and are concise. LGTM.
source/Public/Remove-SqlDscDatabase.ps1 (5)
115-122: System database protection: correct terminating error usage.InvalidOperation with specific ID and localized message is appropriate. LGTM.
132-139: Missing database path: good use of ObjectNotFound and specific ErrorId.Error shape and categorization look right. LGTM.
153-160: System database guard (object parameter set) looks correct.Consistent with the server-object path. LGTM.
219-226: Failure path preserves inner exception and targets the DB object — good.Terminating error construction is correct and actionable. LGTM.
228-231: State restoration is correct.$ErrorActionPreference is restored even on error — good hygiene. LGTM.
tests/Unit/Public/Remove-SqlDscAgentAlert.Tests.ps1 (1)
214-221: Test correctly validates terminating error properties.The test properly captures the error via
-PassThruand validates all essential error properties (exception type, message pattern, and FullyQualifiedErrorId). This aligns with the PR objective to use$PSCmdlet.ThrowTerminatingError()for terminating errors.tests/Unit/Public/New-SqlDscDatabase.Tests.ps1 (2)
136-143: Comprehensive validation of CompatibilityLevel error.The test properly validates all ErrorRecord properties including the category (
InvalidArgument) and target name (Version80), which correctly identifies the invalid parameter value.
146-153: Comprehensive validation of Collation error.The test properly validates all ErrorRecord properties with the correct error ID (
NSD0004) and target name (InvalidCollation).source/Public/Remove-SqlDscAgentAlert.ps1 (2)
120-148: ErrorActionPreference management follows best practices.The code properly saves the original
$ErrorActionPreference, sets it to'Stop'before the operation, and restores it in thefinallyblock. This ensures proper cleanup regardless of success or failure.
136-143: Missing exception in ThrowTerminatingError call.The
ErrorRecord::new()constructor is missing the original exception from the catch block. The pattern should include$_.Exceptionto preserve the exception chain.Apply this diff to include the original exception:
$PSCmdlet.ThrowTerminatingError( [System.Management.Automation.ErrorRecord]::new( - [System.InvalidOperationException]::new($errorMessage, $_.Exception), + ([System.InvalidOperationException]::new($errorMessage, $_.Exception)), 'RSAA0005', # cspell: disable-line [System.Management.Automation.ErrorCategory]::InvalidOperation, $alertObjectToRemove ) )Note: The first parameter expects an Exception object. Wrapping in parentheses ensures the InvalidOperationException constructor result is properly passed.
⛔ Skipped due to learnings
Learnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0 Timestamp: 2025-09-12T13:20:57.155Z Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)tests/Unit/Public/Set-SqlDscDatabase.Tests.ps1 (2)
111-111: Error ID validation correctly added to existing tests.The
-ErrorIdparameter additions align with the corresponding error codes in the production code (SSDD0001, SSDD0002, SSDD0003), ensuring proper validation of terminating error behavior.Also applies to: 169-169, 223-223
282-307: New test context provides comprehensive failure coverage.The test properly validates the terminating error behavior when
Alter()fails, including the error message pattern and error ID (SSDD0004). The mock setup correctly simulates anAlter()failure.source/Public/Set-SqlDscDatabase.ps1 (1)
129-137: Rewrite ErrorRecord calls to use typed exception objects for consistency with codebase patterns.While PowerShell's ErrorRecord constructor accepts strings and automatically converts them to exception objects, the codebase standard is to use typed exception objects. Update the four ErrorRecord calls in Set-SqlDscDatabase.ps1 to match the pattern used throughout similar commands.
Lines 131-135: Use
[System.Management.Automation.ItemNotFoundException]::new($errorMessage)(ObjectNotFound category)Lines 167-171 and 185-189: Use
[System.ArgumentException]::new($errorMessage)(InvalidArgument category)Lines 250-254: Use
[System.InvalidOperationException]::new($errorMessage)(InvalidOperation category)⛔ Skipped due to learnings
Learnt from: johlju PR: dsccommunity/SqlServerDsc#2132 File: source/Public/New-SqlDscLogin.ps1:218-225 Timestamp: 2025-08-17T18:25:52.891Z Learning: PowerShell's System.Management.Automation.ErrorRecord constructor accepts a string as the first parameter and automatically converts it to an exception object internally. No need to manually wrap strings in System.Exception when creating ErrorRecord objects in PowerShell.Learnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0 Timestamp: 2025-09-12T13:20:57.155Z Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)Learnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0 Timestamp: 2025-09-12T13:20:14.850Z Learning: Applies to source/[cC]lasses/**/*.ps1 : For terminating errors, use New-*Exception cmdlets: New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/Public/Remove-SqlDscRole.ps1 (1)
96-153: Add missing string ID suffixes to localized error messages.The localized strings in
source/en-US/SqlServerDsc.strings.psd1are missing required string ID suffixes. Per coding guidelines, localized strings must follow the format:KeyName = Message (PREFIX####).Update the following lines:
- Line 317: Add
(RSDR0001)suffix toRole_NotFound- Line 335: Add
(RSDR0002)suffix toRole_CannotRemoveBuiltIn- Line 334: Add
(RSDR0003)suffix toRole_RemoveFailedExample:
Role_NotFound = Server role '{0}' was not found. (RSDR0001) Role_CannotRemoveBuiltIn = Cannot remove built-in server role '{0}'. (RSDR0002) Role_RemoveFailed = Failed to remove server role '{0}' from instance '{1}'. (RSDR0003)
🧹 Nitpick comments (1)
source/Public/Set-SqlDscDatabase.ps1 (1)
248-256: Preserve the original exception in catch block.In catch blocks, preserve the exception chain by passing the original exception as an inner exception. This maintains valuable diagnostic information for troubleshooting.
Apply this diff to preserve the original exception:
$PSCmdlet.ThrowTerminatingError( [System.Management.Automation.ErrorRecord]::new( - [System.InvalidOperationException]::new($errorMessage), + [System.InvalidOperationException]::new($errorMessage, $_.Exception), 'SSDD0004', # SQL Server Database - Set failed [System.Management.Automation.ErrorCategory]::InvalidOperation, $DatabaseObject ) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
source/Public/Remove-SqlDscRole.ps1(3 hunks)source/Public/Set-SqlDscDatabase.ps1(4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
source/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/**/*.ps1: Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning, and $PSCmdlet.ThrowTerminatingError() messages
Use localized string keys instead of hardcoded strings in script output/messages
Assume and use $script:localizedData for accessing localized strings
When emitting messages, reference $script:localizedData.KeyName and format with the -f operator (e.g., Write-Verbose -Message ($script:localizedData.KeyName -f $value1))
Files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.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/Public/Set-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place 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)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Write-Output; use return instead
Do not use ConvertTo-SecureString -AsPlainText in production code
Do not redefine reserved parameters (Verbose, Debug, etc.)
In...
Files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
source/Public/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Place public commands in source/Public/{CommandName}.ps1
Files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession: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 instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- 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
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # 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...
Files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : For terminating errors use $PSCmdlet.ThrowTerminatingError() (not in classes); include relevant error category and in catch include exception with localized message
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Use Write-Error for non-terminating errors with -Message (localized), -Category, -ErrorId (unique, matches localized ID), -TargetObject; in catch include -Exception; always return after Write-Error
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-09-12T13:20:14.850Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Do not use throw for terminating errors in classes
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-09-12T13:20:14.850Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : For terminating errors, use New-*Exception cmdlets: New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Applied to files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
📚 Learning: 2025-08-17T18:25:52.891Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: source/Public/New-SqlDscLogin.ps1:218-225
Timestamp: 2025-08-17T18:25:52.891Z
Learning: PowerShell's System.Management.Automation.ErrorRecord constructor accepts a string as the first parameter and automatically converts it to an exception object internally. No need to manually wrap strings in System.Exception when creating ErrorRecord objects in PowerShell.
Applied to files:
source/Public/Set-SqlDscDatabase.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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (7)
source/Public/Set-SqlDscDatabase.ps1 (4)
129-137: Proper terminating error implementation for database not found.The ErrorRecord construction is correct and follows DSC Community guidelines. The use of
InvalidOperationExceptionwith the localized error message, error IDSSDD0001, and appropriate category/target is properly implemented.Note: Past review comments suggesting this was incorrect have been superseded by the learning that PowerShell's ErrorRecord constructor properly handles exception objects.
165-173: Proper terminating error implementation for invalid compatibility level.The ErrorRecord construction correctly uses
ArgumentExceptionfor an invalid parameter value, with appropriate error IDSSDD0002and categoryInvalidArgument. The target object properly identifies the problematic compatibility level value.
184-191: Proper terminating error implementation for invalid collation.The ErrorRecord construction follows the correct pattern with
ArgumentException, error IDSSDD0003, and appropriate category/target for an invalid collation parameter.
128-128: Let me verify the complete localization string definitions to ensure they include error IDs:Based on my verification of the codebase, all four localized string keys referenced in
source/Public/Set-SqlDscDatabase.ps1exist insource/en-US/SqlServerDsc.strings.psd1:
Database_NotFound(line 353)Database_InvalidCompatibilityLevel(line 361)Database_InvalidCollation(line 362)Database_SetFailed(line 372)The localization keys are properly defined and accessible via the
$script:localizedDatavariable.source/Public/Remove-SqlDscRole.ps1 (3)
96-104: LGTM! Proper ObjectNotFound error handling.The implementation correctly uses
$PSCmdlet.ThrowTerminatingError()with:
ItemNotFoundExceptionfor role not found scenarioObjectNotFounderror category- Unique error ID
RSDR0001following the naming pattern- Appropriate target object (
$Name)- Localized error message
117-125: LGTM! Proper InvalidOperation error handling.The implementation correctly uses
$PSCmdlet.ThrowTerminatingError()with:
InvalidOperationExceptionfor built-in role removal attemptInvalidOperationerror category- Unique error ID
RSDR0002- Appropriate target object (
$RoleObject)- Localized error message
145-153: LGTM! Proper error handling with exception chaining.The implementation correctly uses
$PSCmdlet.ThrowTerminatingError()in the catch block with:
InvalidOperationExceptionfor operation failure- Inner exception (
$_.Exception) properly preserved for error contextInvalidOperationerror category- Unique error ID
RSDR0003- Appropriate target object (
$RoleObject)- Localized error message
…d and removal failures
…original exception in the error record
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
source/Public/Set-SqlDscDatabase.ps1(4 hunks)source/en-US/SqlServerDsc.strings.psd1(2 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
source/en-US/*.strings.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/en-US/*.strings.psd1: Store command/function localization in source/en-US/{MyModuleName}.strings.psd1
Store class localization in source/en-US/{ResourceClassName}.strings.psd1
Name localization keys as Verb_FunctionName_Action using underscores (e.g., Get_Database_ConnectingToDatabase)
Define strings using ConvertFrom-StringData with entries likeKeyName = Message with {0} placeholder. (PREFIX0001)
Include string IDs in the form (PREFIX####), where PREFIX is initials from the class/function name and numbers are sequential from 0001
Files:
source/en-US/SqlServerDsc.strings.psd1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
source/en-US/SqlServerDsc.strings.psd1source/Public/Set-SqlDscDatabase.ps1
**/*.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
In module manifests, do not use NestedModules for shared commands without specifying RootModule
Files:
source/en-US/SqlServerDsc.strings.psd1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession: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 instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- 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
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/en-US/SqlServerDsc.strings.psd1source/Public/Set-SqlDscDatabase.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # 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...
Files:
source/en-US/SqlServerDsc.strings.psd1source/Public/Set-SqlDscDatabase.ps1
source/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/**/*.ps1: Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning, and $PSCmdlet.ThrowTerminatingError() messages
Use localized string keys instead of hardcoded strings in script output/messages
Assume and use $script:localizedData for accessing localized strings
When emitting messages, reference $script:localizedData.KeyName and format with the -f operator (e.g., Write-Verbose -Message ($script:localizedData.KeyName -f $value1))
Files:
source/Public/Set-SqlDscDatabase.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/Public/Set-SqlDscDatabase.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place 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)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Write-Output; use return instead
Do not use ConvertTo-SecureString -AsPlainText in production code
Do not redefine reserved parameters (Verbose, Debug, etc.)
In...
Files:
source/Public/Set-SqlDscDatabase.ps1
source/Public/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Place public commands in source/Public/{CommandName}.ps1
Files:
source/Public/Set-SqlDscDatabase.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
source/Public/Set-SqlDscDatabase.ps1
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : For terminating errors use $PSCmdlet.ThrowTerminatingError() (not in classes); include relevant error category and in catch include exception with localized message
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Use Write-Error for non-terminating errors with -Message (localized), -Category, -ErrorId (unique, matches localized ID), -TargetObject; in catch include -Exception; always return after Write-Error
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Set $ErrorActionPreference='Stop' immediately before commands using -ErrorAction 'Stop' and restore after; do not use try-catch-finally for this
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2189
File: source/Public/Set-SqlDscDatabaseDefault.ps1:124-130
Timestamp: 2025-09-06T19:12:11.414Z
Learning: In PowerShell functions, when calling commands with -ErrorAction 'Stop', if the command fails it throws a terminating error that stops execution of the calling function entirely, so $ErrorActionPreference restoration only needs to happen after successful completion, not in a finally block.
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-10-12T11:23:30.123Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Use -ErrorAction 'Stop' on commands so failures surface immediately
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-09-12T13:20:14.850Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Do not use throw for terminating errors in classes
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Applied to files:
source/Public/Set-SqlDscDatabase.ps1
📚 Learning: 2025-08-17T18:25:52.891Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: source/Public/New-SqlDscLogin.ps1:218-225
Timestamp: 2025-08-17T18:25:52.891Z
Learning: PowerShell's System.Management.Automation.ErrorRecord constructor accepts a string as the first parameter and automatically converts it to an exception object internally. No need to manually wrap strings in System.Exception when creating ErrorRecord objects in PowerShell.
Applied to files:
source/Public/Set-SqlDscDatabase.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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (5)
source/Public/Set-SqlDscDatabase.ps1 (4)
129-137: LGTM! ErrorRecord construction is correct.The ErrorRecord is now properly constructed with an
InvalidOperationExceptionwrapping the localized error message, addressing the previous review feedback. The error ID, category, and target object are all appropriate for this "database not found" scenario.
165-173: LGTM! ErrorRecord construction is correct.The ErrorRecord properly uses
ArgumentExceptionfor this validation scenario, with appropriate error category (InvalidArgument) and target object. This addresses the previous review feedback.
183-191: LGTM! ErrorRecord construction is correct.The ErrorRecord correctly uses
ArgumentExceptionwith appropriate error category and target object for this collation validation failure, addressing the previous review feedback.
247-256: LGTM! ErrorRecord construction correctly preserves exception chain.The ErrorRecord properly includes the original exception (
$_.Exception) as the inner exception, which preserves the full exception chain for debugging. This correctly addresses the previous review feedback and follows best practices for error handling in catch blocks.source/en-US/SqlServerDsc.strings.psd1 (1)
334-335: LGTM!The error codes are correctly formatted and use the appropriate
RSDRprefix for Remove-SqlDscRole operations. The additions align with the PR's objective to provide proper ErrorRecord details for terminating errors.
…sistency and clarity
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/Public/Get-SqlDscRole.ps1 (1)
76-88: Return immediately after Write-Error to avoid further processingPer guidelines, always return after Write-Error. Add an explicit return of a typed empty array right after Write-Error.
As per coding guidelines
- Write-Error @writeErrorParameters + Write-Error @writeErrorParameters + return [Microsoft.SqlServer.Management.Smo.ServerRole[]] @()
🧹 Nitpick comments (1)
source/Public/Remove-SqlDscRole.ps1 (1)
128-133: Align with standard ShouldProcess + Force/Confirm patternFollow the documented pattern to keep behavior consistent and still bypass prompts with -Force.
As per coding guidelines
- if ($Force.IsPresent -or $PSCmdlet.ShouldProcess($verboseDescriptionMessage, $verboseWarningMessage, $captionMessage)) + # If Force is used without explicit -Confirm, suppress confirmation prompts. + if ($Force.IsPresent -and -not $Confirm) + { + $ConfirmPreference = 'None' + } + + # Use standard variable names for ShouldProcess messages. + $descriptionMessage = $verboseDescriptionMessage + $confirmationMessage = $verboseWarningMessage + + if ($PSCmdlet.ShouldProcess($descriptionMessage, $confirmationMessage, $captionMessage))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
source/Public/Get-SqlDscRole.ps1(1 hunks)source/Public/Remove-SqlDscRole.ps1(3 hunks)source/en-US/SqlServerDsc.strings.psd1(2 hunks)tests/Unit/Public/Get-SqlDscRole.Tests.ps1(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- source/en-US/SqlServerDsc.strings.psd1
🧰 Additional context used
📓 Path-based instructions (11)
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: All public commands, private functions, and classes must have unit tests
All public commands and class-based resources must have integration tests
Use Pester v5 syntax only
Test code only inside Describe blocks
Assertions only in It blocks
Never test verbose messages, debug messages, or parameter binding behavior
Pass all mandatory parameters to avoid prompts
Inside It blocks, assign unused return objects to $null (unless part of a pipeline)
Call the tested entity from within the It blocks
Keep results and assertions in the same It block
Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Avoid unnecessary remove/recreate cycles
One Describe block per file matching the tested entity name
Context descriptions start with 'When'
It descriptions start with 'Should' and must not contain 'when'
Mock variables must be prefixed with 'mock'
Public commands: never use InModuleScope (except for retrieving localized strings)
Private functions and class resources: always use InModuleScope
Each class method gets a separate Context block
Each scenario gets a separate Context block
Use nested Context blocks for complex scenarios
Perform mocking in BeforeAll (use BeforeEach only when required)
Place setup/teardown (BeforeAll/BeforeEach/AfterAll/AfterEach) close to usage
Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Never use Assert-MockCalled; use Should -Invoke instead
Do not use Should -Not -Throw; invoke commands directly
Never add an empty -MockWith block
Omit -MockWith when returning $null
Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Omit the -ModuleName parameter on Pester commands
Never use Mock inside an InModuleScope block
Define variables for -ForEach in a separate BeforeDiscovery near usage
Use -ForEach only on Context and It blocks
Keep variable scope close to the usage c...
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1
tests/Unit/Public/*.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
Public command unit tests must reside in tests/Unit/Public/{Name}.Tests.ps1
Place unit tests for public commands in tests/Unit/Public/{Name}.Tests.ps1
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place 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)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Write-Output; use return instead
Do not use ConvertTo-SecureString -AsPlainText in production code
Do not redefine reserved parameters (Verbose, Debug, etc.)
In...
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1source/Public/Remove-SqlDscRole.ps1source/Public/Get-SqlDscRole.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1source/Public/Remove-SqlDscRole.ps1source/Public/Get-SqlDscRole.ps1
tests/[Uu]nit/**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md)
tests/[Uu]nit/**/*.[Tt]ests.ps1: In unit tests, access localized strings using InModuleScope -ScriptBlock { $script:localizedData.Key }
When mocking files in tests, use the $TestDrive variable for file system operations
All public commands must include parameter set validation tests
Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Use the provided Parameter Set Validation test template to assert command ParameterSetName and full parameter list string; for multiple parameter sets, supply multiple hashtables via -ForEach
Use the Parameter Properties template to assert a parameter is mandatory via $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName']; $parameterInfo.Attributes.Mandatory | Should -BeTrue
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[Uu]nit/**/*.[Tt]ests.ps1: # Unit Tests Guidelines
- Test with localized strings: Use
InModuleScope -ScriptBlock { $script:localizedData.Key }- Mock files: Use
$TestDrivevariable (path to the test drive)- All public commands require parameter set validation tests
- After modifying classes, always run tests in new session (for changes to take effect)
Test Setup Requirements
Use this exact setup block before
Describe:[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName } AfterAll { $PSDefaultParameterValues.Remove('InModuleScope:ModuleName') $PSDefaultParameterValues.Remove('Mock:ModuleNam...
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1source/Public/Remove-SqlDscRole.ps1source/Public/Get-SqlDscRole.ps1
tests/Unit/{Classes,Public,Private}/*.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Add unit tests for all commands, functions, and resources
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession: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 instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- 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
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1source/Public/Remove-SqlDscRole.ps1source/Public/Get-SqlDscRole.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # 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...
Files:
tests/Unit/Public/Get-SqlDscRole.Tests.ps1source/Public/Remove-SqlDscRole.ps1source/Public/Get-SqlDscRole.ps1
source/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/**/*.ps1: Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning, and $PSCmdlet.ThrowTerminatingError() messages
Use localized string keys instead of hardcoded strings in script output/messages
Assume and use $script:localizedData for accessing localized strings
When emitting messages, reference $script:localizedData.KeyName and format with the -f operator (e.g., Write-Verbose -Message ($script:localizedData.KeyName -f $value1))
Files:
source/Public/Remove-SqlDscRole.ps1source/Public/Get-SqlDscRole.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/Public/Remove-SqlDscRole.ps1source/Public/Get-SqlDscRole.ps1
source/Public/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Place public commands in source/Public/{CommandName}.ps1
Files:
source/Public/Remove-SqlDscRole.ps1source/Public/Get-SqlDscRole.ps1
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : For terminating errors use $PSCmdlet.ThrowTerminatingError() (not in classes); include relevant error category and in catch include exception with localized message
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Use Write-Error for non-terminating errors with -Message (localized), -Category, -ErrorId (unique, matches localized ID), -TargetObject; in catch include -Exception; always return after Write-Error
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2189
File: source/Public/Set-SqlDscDatabaseDefault.ps1:124-130
Timestamp: 2025-09-06T19:12:11.414Z
Learning: In PowerShell functions, when calling commands with -ErrorAction 'Stop', if the command fails it throws a terminating error that stops execution of the calling function entirely, so $ErrorActionPreference restoration only needs to happen after successful completion, not in a finally block.
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Set $ErrorActionPreference='Stop' immediately before commands using -ErrorAction 'Stop' and restore after; do not use try-catch-finally for this
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Applied to files:
source/Public/Remove-SqlDscRole.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/Public/Get-SqlDscRole.ps1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/**/*.ps1 : Use localized string keys instead of hardcoded strings in script output/messages
Applied to files:
source/Public/Get-SqlDscRole.ps1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use localized strings for all messages (e.g., Write-Verbose, Write-Error)
Applied to files:
source/Public/Get-SqlDscRole.ps1
📚 Learning: 2025-10-04T21:33:23.022Z
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.
Applied to files:
source/Public/Get-SqlDscRole.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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (4)
tests/Unit/Public/Get-SqlDscRole.Tests.ps1 (1)
127-131: LGTM: message assertion updated to include code (GSDR0001)Change aligns with localized key and ErrorId updates.
source/Public/Remove-SqlDscRole.ps1 (3)
95-105: Good swap to ThrowTerminatingError for not-foundCorrect category, ErrorId, and localized message. Targeting $Name is appropriate for this parameter set.
118-126: Built-in role guard now properly terminatesCorrect use of InvalidOperation category and localized message.
146-153: Removal failure now surfaces original exception with localized contextGood: wraps inner exception, uses InvalidOperation, and terminates via ErrorRecord.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2312 +/- ##
====================================
Coverage 94% 94%
====================================
Files 150 150
Lines 9248 9246 -2
====================================
Hits 8760 8760
+ Misses 488 486 -2
🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…or non-existent role
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
source/Public/New-SqlDscRole.ps1 (1)
96-104: Update ErrorId prefix from NSR to NSDR for New-SqlDscRole function alignment with coding guidelines.The review is correct. Per the established naming convention (first letter of each word), New-SqlDscRole requires the NSDR prefix (N-S-D-R), not NSR. This matches the pattern used throughout the codebase: Remove-SqlDscRole uses RSDR, Set-SqlDscDatabase uses SSDD, etc. Both error IDs at lines 99 and 136 need updating.
source/Public/Remove-SqlDscDatabase.ps1 (1)
213-239: Drop() failure leaves DB in SINGLE_USER when -DropConnections; add best-effort revert and remove EAP togglingIf Drop() throws after setting SINGLE_USER, the database can be left locked in single-user mode. Add a best-effort revert to MULTI_USER before throwing. Also remove EAP toggling here as well.
- try - { - $originalErrorActionPreference = $ErrorActionPreference - - $ErrorActionPreference = 'Stop' - - $DatabaseObject.Drop() - - Write-Verbose -Message ($script:localizedData.Database_Removed -f $Name) - } - catch - { - $errorMessage = $script:localizedData.Database_RemoveFailed -f $Name, $DatabaseObject.Parent.InstanceName - - $PSCmdlet.ThrowTerminatingError( - [System.Management.Automation.ErrorRecord]::new( - [System.InvalidOperationException]::new($errorMessage, $_.Exception), - 'RSDD0005', # cspell: disable-line - [System.Management.Automation.ErrorCategory]::InvalidOperation, - $DatabaseObject - ) - ) - } - finally - { - $ErrorActionPreference = $originalErrorActionPreference - } + try + { + $DatabaseObject.Drop() + + Write-Verbose -Message ($script:localizedData.Database_Removed -f $Name) + } + catch + { + if ($DropConnections.IsPresent) + { + try + { + $DatabaseObject.UserAccess = [Microsoft.SqlServer.Management.Smo.DatabaseUserAccess]::Multiple + $DatabaseObject.Alter([Microsoft.SqlServer.Management.Smo.TerminationClause]::RollbackTransactionsImmediately) + } + catch + { + Write-Debug -Message ($script:localizedData.Database_RevertMultiUserFailed -f $Name) + } + } + + $errorMessage = $script:localizedData.Database_RemoveFailed -f $Name, $DatabaseObject.Parent.InstanceName + + $PSCmdlet.ThrowTerminatingError( + [System.Management.Automation.ErrorRecord]::new( + [System.InvalidOperationException]::new($errorMessage, $_.Exception), + 'RSDD0005', # cspell: disable-line + [System.Management.Automation.ErrorCategory]::InvalidOperation, + $DatabaseObject + ) + ) + }Add the new localized string key Database_RevertMultiUserFailed. As per coding guidelines.
🧹 Nitpick comments (7)
source/Public/New-SqlDscRole.ps1 (2)
110-110: Align ShouldProcess usage with style: variable names and no Verbose in block.
- Rename variables to $descriptionMessage/$confirmationMessage/$captionMessage.
- Avoid Write-Verbose inside the ShouldProcess block; rely on the ShouldProcess messages or emit status via Write-Information if needed. [As per coding guidelines]
114-115: Prefer ::new() over New-Object for .NET types.Use the static ctor for SMO types. [As per coding guidelines]
- $serverRole = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ServerRole -ArgumentList $ServerObject, $Name + $serverRole = [Microsoft.SqlServer.Management.Smo.ServerRole]::new($ServerObject, $Name)source/Public/Remove-SqlDscRole.ps1 (1)
140-140: ShouldProcess style nits: standard var names; avoid Verbose inside.
- Rename to $descriptionMessage/$confirmationMessage/$captionMessage.
- Keep the block focused on the state change; drop/move Write-Verbose from inside it. [As per coding guidelines]
source/Public/Remove-SqlDscDatabase.ps1 (4)
121-130: Unify ErrorId for the same condition ("cannot remove system database")Two different ErrorIds are used for the same error condition (RSDD0001 vs RSDD0003). Prefer a single ErrorId to simplify localization, testing, and troubleshooting.
Apply this diff to reuse RSDD0001 in both branches:
- 'RSDD0003', # cspell: disable-line + 'RSDD0001', # cspell: disable-lineUpdate unit tests/localized docs accordingly. As per coding guidelines.
Also applies to: 161-168
176-176: Align variable names with the ShouldProcess patternUse the canonical variable names $descriptionMessage/$confirmationMessage/$captionMessage for clarity and to match our pattern.
- $verboseDescriptionMessage = $script:localizedData.Database_Remove_ShouldProcessVerboseDescription -f $Name, $DatabaseObject.Parent.InstanceName - $verboseWarningMessage = $script:localizedData.Database_Remove_ShouldProcessVerboseWarning -f $Name + $descriptionMessage = $script:localizedData.Database_Remove_ShouldProcessVerboseDescription -f $Name, $DatabaseObject.Parent.InstanceName + $confirmationMessage = $script:localizedData.Database_Remove_ShouldProcessVerboseWarning -f $Name @@ - if ($PSCmdlet.ShouldProcess($verboseDescriptionMessage, $verboseWarningMessage, $captionMessage)) + if ($PSCmdlet.ShouldProcess($descriptionMessage, $confirmationMessage, $captionMessage))As per coding guidelines.
178-209: Remove ErrorActionPreference toggling around SMO methods$ErrorActionPreference only affects PowerShell cmdlets with -ErrorAction, not .NET/SMO methods. The try/finally used solely to save/restore EAP is unnecessary noise.
Apply this diff:
- try - { - $originalErrorActionPreference = $ErrorActionPreference - - $ErrorActionPreference = 'Stop' - - $DatabaseObject.UserAccess = 'Single' - $DatabaseObject.Alter([Microsoft.SqlServer.Management.Smo.TerminationClause]::RollbackTransactionsImmediately) - } + try + { + $DatabaseObject.UserAccess = [Microsoft.SqlServer.Management.Smo.DatabaseUserAccess]::Single + $DatabaseObject.Alter([Microsoft.SqlServer.Management.Smo.TerminationClause]::RollbackTransactionsImmediately) + } catch { $errorMessage = $script:localizedData.Database_DropConnectionsFailed -f $Name @@ - finally - { - $ErrorActionPreference = $originalErrorActionPreference - }As per retrieved learnings and coding guidelines.
181-181: Avoid Write-Verbose inside the ShouldProcess blockGuideline: inside $PSCmdlet.ShouldProcess { ... } avoid Write-Verbose. Either remove these or emit pre-/post-action messages outside the block.
As per coding guidelines.Also applies to: 211-211, 221-221
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
source/Public/Get-SqlDscRole.ps1(2 hunks)source/Public/New-SqlDscRole.ps1(2 hunks)source/Public/Remove-SqlDscDatabase.ps1(5 hunks)source/Public/Remove-SqlDscRole.ps1(4 hunks)source/Public/Set-SqlDscDatabase.ps1(5 hunks)
🧰 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 Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession: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 instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- 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
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Get-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscRole.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # 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...
Files:
source/Public/Set-SqlDscDatabase.ps1source/Public/Get-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscRole.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/Public/Set-SqlDscDatabase.ps1source/Public/Get-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscRole.ps1
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : For terminating errors use $PSCmdlet.ThrowTerminatingError() (not in classes); include relevant error category and in catch include exception with localized message
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Use Write-Error for non-terminating errors with -Message (localized), -Category, -ErrorId (unique, matches localized ID), -TargetObject; in catch include -Exception; always return after Write-Error
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2189
File: source/Public/Set-SqlDscDatabaseDefault.ps1:124-130
Timestamp: 2025-09-06T19:12:11.414Z
Learning: In PowerShell functions, when calling commands with -ErrorAction 'Stop', if the command fails it throws a terminating error that stops execution of the calling function entirely, so $ErrorActionPreference restoration only needs to happen after successful completion, not in a finally block.
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Set $ErrorActionPreference='Stop' immediately before commands using -ErrorAction 'Stop' and restore after; do not use try-catch-finally for this
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
📚 Learning: 2025-08-17T18:25:52.891Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: source/Public/New-SqlDscLogin.ps1:218-225
Timestamp: 2025-08-17T18:25:52.891Z
Learning: PowerShell's System.Management.Automation.ErrorRecord constructor accepts a string as the first parameter and automatically converts it to an exception object internally. No need to manually wrap strings in System.Exception when creating ErrorRecord objects in PowerShell.
Applied to files:
source/Public/Set-SqlDscDatabase.ps1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Applied to files:
source/Public/Set-SqlDscDatabase.ps1
📚 Learning: 2025-09-16T16:34:27.319Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Implement Force parameter pattern: if ($Force.IsPresent -and -not $Confirm) { $ConfirmPreference = 'None' }
Applied to files:
source/Public/Set-SqlDscDatabase.ps1source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscRole.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-SqlDscDatabase.ps1source/Public/New-SqlDscRole.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-SqlDscDatabase.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/Public/Get-SqlDscRole.ps1
📚 Learning: 2025-09-16T16:34:27.319Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Follow the ShouldProcess message variables pattern ($descriptionMessage, $confirmationMessage, $captionMessage)
Applied to files:
source/Public/New-SqlDscRole.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). (33)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Reporting Services Integration SQL2022_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Reporting Services Integration SQL2022_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Reporting Services Integration SQL2019_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Reporting Services (dbatools) Integration SQL2022_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Reporting Services Integration SQL2019_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Reporting Services (dbatools) Integration SQL2019_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Reporting Services Integration SQL2017_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Reporting Services (dbatools) Integration SQL2017_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server (dbatools) Integration SQL2022_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server (dbatools) Integration SQL2019_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server (dbatools) Integration SQL2017_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2022_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2022_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2022_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2022_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2019_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2019_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2019_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2019_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2017_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2017_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2022_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2022_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2019_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2019_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Power BI Report Server Integration PowerBI_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2017_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Power BI Report Server Integration PowerBI_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Publish Code Coverage)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (13)
source/Public/Get-SqlDscRole.ps1 (1)
76-89: LGTM! Proper implementation of non-terminating error pattern.The changes correctly implement the coding guidelines:
- Localization key renamed to follow
Verb_FunctionName_ActionpatternWrite-Errorincludes all required parameters (Message, Category, ErrorId, TargetObject)returnstatement properly added afterWrite-Errorto prevent further processing- ErrorId
GSDR0001follows the correct PREFIX pattern (Get-SqlDscRole → GSDR)The use of a non-terminating error is appropriate for a Get command, allowing pipeline operations to continue when a role is not found.
Based on coding guidelines.
source/Public/Set-SqlDscDatabase.ps1 (5)
111-117: LGTM! Force parameter pattern correctly implemented.The begin block properly implements the Force parameter pattern, suppressing confirmations when
-Forceis used while respecting explicit-Confirm:$true.Based on coding guidelines.
138-145: LGTM! Terminating error correctly constructed.The ErrorRecord construction is correct:
- Appropriate exception type (
InvalidOperationException) for database not found- Correct error category (
InvalidOperation)- Proper error ID (
SSDD0001)- Target object correctly set to
$Name
174-181: LGTM! Input validation error correctly handled.The ErrorRecord construction is appropriate:
ArgumentExceptioncorrectly used for invalid parameter value- Error category (
InvalidArgument) matches the validation failure- Target object properly set to
$CompatibilityLevel- Sequential error ID (
SSDD0002)
192-199: LGTM! Collation validation error correctly implemented.The ErrorRecord follows the same correct pattern:
ArgumentExceptionfor invalid parameter validation- Appropriate error category (
InvalidArgument)- Target object set to
$Collation- Sequential error ID (
SSDD0003)
257-264: LGTM! Exception chain correctly preserved in catch block.The ErrorRecord construction properly:
- Uses
InvalidOperationExceptionfor the operation failure- Preserves the original exception via
$_.Exceptionas the inner exception (line 259)- Sets appropriate error category (
InvalidOperation)- Targets
$DatabaseObject- Uses sequential error ID (
SSDD0004)This correctly maintains the full exception chain for debugging.
Based on coding guidelines.
source/Public/New-SqlDscRole.ps1 (1)
73-79: Force/Confirm pattern looks correct.Respects explicit -Confirm and suppresses prompts only when -Force and not -Confirm. Scope-local ConfirmPreference change. [Based on learnings]
source/Public/Remove-SqlDscRole.ps1 (4)
78-84: Force/Confirm handling LGTM.Pattern matches guidance; -Force doesn’t override explicit -Confirm. [Based on learnings]
126-133: Built-in role protection LGTM.InvalidOperation with target RoleObject and RSDR0002 is appropriate.
154-161: Failure path terminating error LGTM.Wraps original exception, InvalidOperation category, targets RoleObject, ErrorId RSDR0003.
103-112: All verification requirements confirmed; code is correct.The localized key
Remove_SqlDscRole_NotFoundand error IDRSDR0001both exist in the strings file (line 334 of source/en-US/SqlServerDsc.strings.psd1). The code snippet at lines 103-112 correctly uses both resources, pairsItemNotFoundExceptionwithObjectNotFoundcategory appropriately, and targets theNameparameter. No duplicate error IDs exist in the codebase. All naming conventions are followed.source/Public/Remove-SqlDscDatabase.ps1 (2)
97-104: Force/Confirm pattern looks goodSetting ConfirmPreference to 'None' when -Force and not -Confirm is aligned with the Force pattern.
If other commands in this PR follow the same pattern, keep it consistent across all. As per coding guidelines.
140-147: Good NotFound handlingItemNotFoundException with ObjectNotFound category and a localized message via ThrowTerminatingError is correct.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
tests/Unit/Stubs/SMO.cs (1)
166-171: LGTM! Clean stub addition for database user access modes.The enum follows SMO conventions and provides the necessary stub for testing database user access functionality. The values correctly represent SQL Server's database user access modes (MULTI_USER, SINGLE_USER, RESTRICTED_USER).
Consider adding a "Used by:" documentation comment for consistency with other enums in this file:
+ // TypeName: Microsoft.SqlServer.Management.Smo.DatabaseUserAccess + // Used by: + // New-SqlDscDatabase.Tests.ps1 + // Set-SqlDscDatabase.Tests.ps1 public enum DatabaseUserAccess : intsource/Public/Remove-SqlDscRole.ps1 (2)
78-84: Force pattern: looks correct; optional hardening for -Confirm detectionPattern aligns with our guidance. Optionally, avoid depending on
$Confirmbeing auto-bound by checking$PSBoundParametersinstead.- if ($Force.IsPresent -and -not $Confirm) + if ($Force.IsPresent -and -not $PSBoundParameters.ContainsKey('Confirm')) { $ConfirmPreference = 'None' }As per coding guidelines.
101-112: NotFound path uses proper terminating error; minor style nit for null-checkConstruction of
ErrorRecord(ItemNotFound, RSDR0001, target$Name) is spot on. Prefer explicit null comparison per style guide.- if (-not $RoleObject) + if ($null -eq $RoleObject) { $errorMessage = $script:localizedData.Remove_SqlDscRole_NotFound -f $Name $PSCmdlet.ThrowTerminatingError( [System.Management.Automation.ErrorRecord]::new( [System.Management.Automation.ItemNotFoundException]::new($errorMessage), 'RSDR0001', # cspell: disable-line [System.Management.Automation.ErrorCategory]::ObjectNotFound, $Name ) ) }As per coding guidelines.
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1 (2)
82-85: Also assert ErrorId for the not-found caseThe implementation throws
RSDD0002for missing DBs; assert it to fully verify semantics.- { Remove-SqlDscDatabase -ServerObject $mockServerObject -Name 'NonExistentDatabase' -Force } | - Should -Throw -ExpectedMessage '*not found*' + { Remove-SqlDscDatabase -ServerObject $mockServerObject -Name 'NonExistentDatabase' -Force } | + Should -Throw -ExpectedMessage '*not found*' -ErrorId 'RSDD0002,Remove-SqlDscDatabase'
52-221: Prefer localized strings in assertionsFetch messages using
InModuleScope { $script:localizedData.Key }instead of hardcoded substrings to avoid locale brittleness.Example:
$expected = InModuleScope -ScriptBlock { $script:localizedData.Database_CannotRemoveSystem } -ModuleName $script:dscModuleName { Remove-SqlDscDatabase -ServerObject $mockServerObject -Name 'master' -Force } | Should -Throw -ExpectedMessage ("*{0}*" -f $expected)As per coding guidelines.
source/Public/Remove-SqlDscDatabase.ps1 (2)
152-156: Align ShouldProcess string keys with patternUse
...ShouldProcessDescriptionand...ShouldProcessConfirmationkeys (no “Verbose”/“Warning” suffixes) to match the documented pattern.-$descriptionMessage = $script:localizedData.Database_Remove_ShouldProcessVerboseDescription -f $Name, $DatabaseObject.Parent.InstanceName -$confirmationMessage = $script:localizedData.Database_Remove_ShouldProcessVerboseWarning -f $Name +$descriptionMessage = $script:localizedData.Database_Remove_ShouldProcessDescription -f $Name, $DatabaseObject.Parent.InstanceName +$confirmationMessage = $script:localizedData.Database_Remove_ShouldProcessConfirmation -f $NameAs per coding guidelines.
158-181: Avoid Write-Verbose inside ShouldProcessReplace with Write-Information for user-facing status, or move messaging outside the block.
- Write-Verbose -Message ($script:localizedData.Database_DroppingConnections -f $Name) + Write-Information -MessageData ($script:localizedData.Database_DroppingConnections -f $Name)As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
source/Public/New-SqlDscRole.ps1(3 hunks)source/Public/Remove-SqlDscDatabase.ps1(2 hunks)source/Public/Remove-SqlDscRole.ps1(3 hunks)source/en-US/SqlServerDsc.strings.psd1(2 hunks)tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1(1 hunks)tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1(2 hunks)tests/Unit/Stubs/SMO.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- source/en-US/SqlServerDsc.strings.psd1
🧰 Additional context used
📓 Path-based instructions (13)
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: All public commands, private functions, and classes must have unit tests
All public commands and class-based resources must have integration tests
Use Pester v5 syntax only
Test code only inside Describe blocks
Assertions only in It blocks
Never test verbose messages, debug messages, or parameter binding behavior
Pass all mandatory parameters to avoid prompts
Inside It blocks, assign unused return objects to $null (unless part of a pipeline)
Call the tested entity from within the It blocks
Keep results and assertions in the same It block
Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Avoid unnecessary remove/recreate cycles
One Describe block per file matching the tested entity name
Context descriptions start with 'When'
It descriptions start with 'Should' and must not contain 'when'
Mock variables must be prefixed with 'mock'
Public commands: never use InModuleScope (except for retrieving localized strings)
Private functions and class resources: always use InModuleScope
Each class method gets a separate Context block
Each scenario gets a separate Context block
Use nested Context blocks for complex scenarios
Perform mocking in BeforeAll (use BeforeEach only when required)
Place setup/teardown (BeforeAll/BeforeEach/AfterAll/AfterEach) close to usage
Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Never use Assert-MockCalled; use Should -Invoke instead
Do not use Should -Not -Throw; invoke commands directly
Never add an empty -MockWith block
Omit -MockWith when returning $null
Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Omit the -ModuleName parameter on Pester commands
Never use Mock inside an InModuleScope block
Define variables for -ForEach in a separate BeforeDiscovery near usage
Use -ForEach only on Context and It blocks
Keep variable scope close to the usage c...
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1
tests/Unit/Public/*.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
Public command unit tests must reside in tests/Unit/Public/{Name}.Tests.ps1
Place unit tests for public commands in tests/Unit/Public/{Name}.Tests.ps1
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place 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)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Write-Output; use return instead
Do not use ConvertTo-SecureString -AsPlainText in production code
Do not redefine reserved parameters (Verbose, Debug, etc.)
In...
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
tests/[Uu]nit/**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md)
tests/[Uu]nit/**/*.[Tt]ests.ps1: In unit tests, access localized strings using InModuleScope -ScriptBlock { $script:localizedData.Key }
When mocking files in tests, use the $TestDrive variable for file system operations
All public commands must include parameter set validation tests
Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Use the provided Parameter Set Validation test template to assert command ParameterSetName and full parameter list string; for multiple parameter sets, supply multiple hashtables via -ForEach
Use the Parameter Properties template to assert a parameter is mandatory via $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName']; $parameterInfo.Attributes.Mandatory | Should -BeTrue
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[Uu]nit/**/*.[Tt]ests.ps1: # Unit Tests Guidelines
- Test with localized strings: Use
InModuleScope -ScriptBlock { $script:localizedData.Key }- Mock files: Use
$TestDrivevariable (path to the test drive)- All public commands require parameter set validation tests
- After modifying classes, always run tests in new session (for changes to take effect)
Test Setup Requirements
Use this exact setup block before
Describe:[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName } AfterAll { $PSDefaultParameterValues.Remove('InModuleScope:ModuleName') $PSDefaultParameterValues.Remove('Mock:ModuleNam...
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
tests/Unit/{Classes,Public,Private}/*.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Add unit tests for all commands, functions, and resources
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession: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 instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- 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
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1tests/Unit/Stubs/SMO.cstests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # 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...
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
tests/Integration/Commands/*.Integration.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
tests/Integration/Commands/*.Integration.Tests.ps1: Place integration tests for public commands in tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Add integration tests for all public commands (and resources)Place command integration tests at tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Files:
tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md)
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: Do not use mocking in integration tests; run against a real environment
Cover all scenarios and code paths in integration tests
Use Get-ComputerName to determine computer names in CI
Avoid using -ExpectedMessage with Should -Throw assertions
Call commands with -Force where applicable to avoid prompting
Use -ErrorAction 'Stop' on commands so failures surface immediately
Include the required Pester setup block (SuppressMessage param, BeforeDiscovery loading DscResource.Test, and BeforeAll importing the module)
Files:
tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests GuidelinesRequirements
- Location Commands:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1- Location Resources:
tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1- No mocking - real environment only
- Cover all scenarios and code paths
- Use
Get-ComputerNamefor computer names in CI- Avoid
ExpectedMessageforShould -Throwassertions- Only run integration tests in CI unless explicitly instructed.
- Call commands with
-Forceparameter where applicable (avoids prompting).- Use
-ErrorAction 'Stop'on commands so failures surface immediatelyRequired Setup Block
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' }
Files:
tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1
source/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/**/*.ps1: Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning, and $PSCmdlet.ThrowTerminatingError() messages
Use localized string keys instead of hardcoded strings in script output/messages
Assume and use $script:localizedData for accessing localized strings
When emitting messages, reference $script:localizedData.KeyName and format with the -f operator (e.g., Write-Verbose -Message ($script:localizedData.KeyName -f $value1))
Files:
source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.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/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
source/Public/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Place public commands in source/Public/{CommandName}.ps1
Files:
source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscDatabase.ps1source/Public/Remove-SqlDscRole.ps1
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : For terminating errors use $PSCmdlet.ThrowTerminatingError() (not in classes); include relevant error category and in catch include exception with localized message
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Use Write-Error for non-terminating errors with -Message (localized), -Category, -ErrorId (unique, matches localized ID), -TargetObject; in catch include -Exception; always return after Write-Error
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
📚 Learning: 2025-09-23T10:20:59.832Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-23T10:20:59.832Z
Learning: Applies to tests/Stubs/SMO.cs : Unit tests that cover classes/commands using SMO types must have those SMO types stubbed in tests/Stubs/SMO.cs
Applied to files:
tests/Unit/Stubs/SMO.cs
📚 Learning: 2025-10-12T11:23:30.123Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-10-12T11:23:30.123Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Avoid using -ExpectedMessage with Should -Throw assertions
Applied to files:
tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:34:27.319Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Implement Force parameter pattern: if ($Force.IsPresent -and -not $Confirm) { $ConfirmPreference = 'None' }
Applied to files:
source/Public/New-SqlDscRole.ps1source/Public/Remove-SqlDscRole.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/New-SqlDscRole.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 (16)
source/Public/New-SqlDscRole.ps1 (4)
73-79: LGTM! Force parameter pattern correctly implemented.The begin block properly implements the Force parameter interaction pattern, ensuring confirmations are suppressed when
-Forceis present unless the user explicitly specifies-Confirm:$true.Based on learnings
99-99: LGTM! Error code follows naming convention.The error code
NSDR0001correctly follows the PREFIX#### format where NSDR is derived from New-SqlDscRole.As per coding guidelines
106-110: LGTM! ShouldProcess pattern correctly implemented.The ShouldProcess implementation follows the required pattern with properly named message variables (
descriptionMessage,confirmationMessage,captionMessage) and correct localization usage.As per coding guidelines
132-132: LGTM! Error code maintains consistency.The error code
NSDR0002maintains consistency with the naming convention and sequential numbering used throughout the file.As per coding guidelines
tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1 (3)
1-30: LGTM! Required setup block is correct.The integration test setup follows the required pattern exactly, including the
SuppressMessageAttribute, properBeforeDiscoveryblock for loadingDscResource.Test, andBeforeAllblock for importing the module under test.
32-50: LGTM! Test initialization is properly structured.The test setup correctly establishes a connection to a real SQL Server instance with proper credentials and includes appropriate cleanup in
AfterAllto disconnect the server object.
52-126: LGTM! Comprehensive test coverage with proper structure.The test cases provide excellent coverage of various scenarios:
- Getting all roles vs. specific roles
- System roles vs. user-created roles
- Error handling (both throwing and silently continuing)
- Refresh parameter behavior
- Pipeline input
The tests follow Pester v5 syntax correctly with proper naming conventions and no mocking (appropriate for integration tests).
source/Public/Remove-SqlDscRole.ps1 (3)
126-133: Built-in role guard: correct category/ID/targetGood use of InvalidOperation, ID
RSDR0002, and targeting the role object. No issues.
136-141: ShouldProcess message pattern is correct and localizedVariables and call signature follow the required pattern. Ensure the caption string has no trailing period and the keys exist in the strings file.
As per coding guidelines.
150-157: Failure path preserves original exception; correct terminating errorWrapping original exception, using
InvalidOperation, and IDRSDR0003is appropriate. Consider adding/confirming unit tests that assert:
$_.CategoryInfo.CategoryandFullyQualifiedErrorId- Exception message localization and inner exception type
- Behavior of
-WhatIfand-Forcewith/without-ConfirmBased on learnings.
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1 (2)
88-90: Good: asserting FullyQualifiedErrorId for system DB pathAdding
-ErrorId 'RSDD0001,Remove-SqlDscDatabase'ensures the terminating error is validated precisely.
124-126: Good: ErrorId asserted for DatabaseObject system DB pathMirrors the command’s
RSDD0001behavior via ThrowTerminatingError.source/Public/Remove-SqlDscDatabase.ps1 (4)
97-104: Force pattern correctly disables confirmationBegin-block handles
-ForcewithConfirmPreference = 'None'when-not $Confirm. Matches module guidance. As per coding guidelines.
118-129: Correct terminating error for not-found databaseUses localized message,
ItemNotFoundException,ObjectNotFoundcategory, and ErrorIdRSDD0002. TargetObject is the name, which is acceptable. Based on learnings.
137-150: Correct system-database guard as terminating errorAppropriate
InvalidOperation,RSDD0001, and localized message.
183-199: Good failure wrapping for drop operationLocalized message, inner exception preservation,
InvalidOperationcategory, andRSDD0005ErrorId are spot on.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1 (3)
83-85: Good use of localized strings and ErrorId assertions.Fetching the message via InModuleScope and asserting ErrorId aligns with the updated terminating-error semantics. As a stability tweak, you can drop -ExpectedMessage and assert only -ErrorId to reduce brittleness from message text changes.
Also applies to: 88-88
92-95: Same here—test is aligned; consider simplifying assertion.Pattern is correct. Optionally remove -ExpectedMessage and rely on -ErrorId only for resilience.
Also applies to: 97-97
132-135: Consistent with new behavior; consider adding negative-path coverage.This assertion is fine. Consider adding unit tests for:
- Drop-connections failure → RSDD0004
- Drop failure → RSDD0005
I can scaffold tests if helpful.
Also applies to: 137-137
source/Public/Remove-SqlDscDatabase.ps1 (2)
159-166: Avoid Write-Verbose inside ShouldProcess; prefer Information or move outside.Guideline says to avoid Write-Verbose inside the ShouldProcess block. Suggest emitting a user-facing status via Write-Information instead.
- Write-Verbose -Message ($script:localizedData.Database_DroppingConnections -f $Name) + Write-Information -MessageData ($script:localizedData.Database_DroppingConnections -f $Name)Also applies to: 161-161
183-199: Robust failure handling; optional improvement to restore access mode on failure.Catch converts to a terminating InvalidOperation with inner exception and RSDD0005—good. Optional: best‑effort revert UserAccess back to Multiple if Drop() fails after switching to Single (only when -DropConnections) to avoid leaving the DB in single‑user state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
source/Public/Remove-SqlDscDatabase.ps1(2 hunks)source/en-US/SqlServerDsc.strings.psd1(2 hunks)tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1(1 hunks)tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1(2 hunks)tests/Unit/Stubs/SMO.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1
🧰 Additional context used
📓 Path-based instructions (13)
source/en-US/*.strings.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/en-US/*.strings.psd1: Store command/function localization in source/en-US/{MyModuleName}.strings.psd1
Store class localization in source/en-US/{ResourceClassName}.strings.psd1
Name localization keys as Verb_FunctionName_Action using underscores (e.g., Get_Database_ConnectingToDatabase)
Define strings using ConvertFrom-StringData with entries likeKeyName = Message with {0} placeholder. (PREFIX0001)
Include string IDs in the form (PREFIX####), where PREFIX is initials from the class/function name and numbers are sequential from 0001
Files:
source/en-US/SqlServerDsc.strings.psd1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
source/en-US/SqlServerDsc.strings.psd1tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1
**/*.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
In module manifests, do not use NestedModules for shared commands without specifying RootModule
Files:
source/en-US/SqlServerDsc.strings.psd1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession: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 instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- 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
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/en-US/SqlServerDsc.strings.psd1tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1tests/Unit/Stubs/SMO.cs
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # 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...
Files:
source/en-US/SqlServerDsc.strings.psd1tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: All public commands, private functions, and classes must have unit tests
All public commands and class-based resources must have integration tests
Use Pester v5 syntax only
Test code only inside Describe blocks
Assertions only in It blocks
Never test verbose messages, debug messages, or parameter binding behavior
Pass all mandatory parameters to avoid prompts
Inside It blocks, assign unused return objects to $null (unless part of a pipeline)
Call the tested entity from within the It blocks
Keep results and assertions in the same It block
Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Avoid unnecessary remove/recreate cycles
One Describe block per file matching the tested entity name
Context descriptions start with 'When'
It descriptions start with 'Should' and must not contain 'when'
Mock variables must be prefixed with 'mock'
Public commands: never use InModuleScope (except for retrieving localized strings)
Private functions and class resources: always use InModuleScope
Each class method gets a separate Context block
Each scenario gets a separate Context block
Use nested Context blocks for complex scenarios
Perform mocking in BeforeAll (use BeforeEach only when required)
Place setup/teardown (BeforeAll/BeforeEach/AfterAll/AfterEach) close to usage
Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Never use Assert-MockCalled; use Should -Invoke instead
Do not use Should -Not -Throw; invoke commands directly
Never add an empty -MockWith block
Omit -MockWith when returning $null
Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Omit the -ModuleName parameter on Pester commands
Never use Mock inside an InModuleScope block
Define variables for -ForEach in a separate BeforeDiscovery near usage
Use -ForEach only on Context and It blocks
Keep variable scope close to the usage c...
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
tests/Unit/Public/*.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
Public command unit tests must reside in tests/Unit/Public/{Name}.Tests.ps1
Place unit tests for public commands in tests/Unit/Public/{Name}.Tests.ps1
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place 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)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Write-Output; use return instead
Do not use ConvertTo-SecureString -AsPlainText in production code
Do not redefine reserved parameters (Verbose, Debug, etc.)
In...
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1
tests/[Uu]nit/**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md)
tests/[Uu]nit/**/*.[Tt]ests.ps1: In unit tests, access localized strings using InModuleScope -ScriptBlock { $script:localizedData.Key }
When mocking files in tests, use the $TestDrive variable for file system operations
All public commands must include parameter set validation tests
Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Use the provided Parameter Set Validation test template to assert command ParameterSetName and full parameter list string; for multiple parameter sets, supply multiple hashtables via -ForEach
Use the Parameter Properties template to assert a parameter is mandatory via $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName']; $parameterInfo.Attributes.Mandatory | Should -BeTrue
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[Uu]nit/**/*.[Tt]ests.ps1: # Unit Tests Guidelines
- Test with localized strings: Use
InModuleScope -ScriptBlock { $script:localizedData.Key }- Mock files: Use
$TestDrivevariable (path to the test drive)- All public commands require parameter set validation tests
- After modifying classes, always run tests in new session (for changes to take effect)
Test Setup Requirements
Use this exact setup block before
Describe:[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName } AfterAll { $PSDefaultParameterValues.Remove('InModuleScope:ModuleName') $PSDefaultParameterValues.Remove('Mock:ModuleNam...
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1source/Public/Remove-SqlDscDatabase.ps1
tests/Unit/{Classes,Public,Private}/*.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Add unit tests for all commands, functions, and resources
Files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
source/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/**/*.ps1: Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning, and $PSCmdlet.ThrowTerminatingError() messages
Use localized string keys instead of hardcoded strings in script output/messages
Assume and use $script:localizedData for accessing localized strings
When emitting messages, reference $script:localizedData.KeyName and format with the -f operator (e.g., Write-Verbose -Message ($script:localizedData.KeyName -f $value1))
Files:
source/Public/Remove-SqlDscDatabase.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/Public/Remove-SqlDscDatabase.ps1
source/Public/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Place public commands in source/Public/{CommandName}.ps1
Files:
source/Public/Remove-SqlDscDatabase.ps1
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : For terminating errors use $PSCmdlet.ThrowTerminatingError() (not in classes); include relevant error category and in catch include exception with localized message
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Use Write-Error for non-terminating errors with -Message (localized), -Category, -ErrorId (unique, matches localized ID), -TargetObject; in catch include -Exception; always return after Write-Error
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
📚 Learning: 2025-10-12T11:23:30.123Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-10-12T11:23:30.123Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Avoid using -ExpectedMessage with Should -Throw assertions
Applied to files:
tests/Unit/Public/Remove-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-09-23T10:20:59.832Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-23T10:20:59.832Z
Learning: Applies to tests/Stubs/SMO.cs : Unit tests that cover classes/commands using SMO types must have those SMO types stubbed in tests/Stubs/SMO.cs
Applied to files:
tests/Unit/Stubs/SMO.cs
⏰ 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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (10)
tests/Unit/Stubs/SMO.cs (1)
166-176: LGTM! Proper stub enum addition.The
DatabaseUserAccessenum is correctly defined with appropriate values that match SQL Server's database user access modes (Multiple, Single, Restricted). The stub follows the established pattern in the file and will properly support testing for the affected cmdlets.source/Public/Remove-SqlDscDatabase.ps1 (6)
97-104: Force pattern implemented correctly.Setting ConfirmPreference to 'None' when -Force and not -Confirm matches the guideline.
120-129: Terminating not‑found error is correct and localized.Using ItemNotFoundException, ObjectNotFound category, target = Name, and RSDD0002 looks right.
137-150: System-database guard is correct.Appropriate InvalidOperation with RSDD0001; good early exit.
152-157: ShouldProcess messages follow the required pattern.description/confirmation/caption variables and call ordering match the style guideline.
163-181: Drop-connections failure path looks good.Throws terminating InvalidOperation with inner exception and RSDD0004; target object is DatabaseObject—good.
152-156: FQID shape verified—all error paths correctly use RSDD000x pattern.All four
ThrowTerminatingErrorcalls in the file use ErrorIds that follow the required pattern:
- Line 125:
RSDD0002- Line 145:
RSDD0001- Line 175:
RSDD0004- Line 194:
RSDD0005When thrown from
Remove-SqlDscDatabase, PowerShell automatically constructs the FullyQualifiedErrorId asRSDD000x,Remove-SqlDscDatabase, matching the test assertion shape.source/en-US/SqlServerDsc.strings.psd1 (3)
317-317: Prefix fix for Get-SqlDscRole not‑found looks good.GSDR0001 under the Get‑SqlDscRole section resolves the earlier prefix mismatch.
323-325: ShouldProcess text normalization is consistent.Description/Confirmation keys for role creation match new pattern.
330-335: Remove‑SqlDscRole messages are correctly aligned with RSDR prefix.Verification confirms all localization keys are properly defined in
source/en-US/SqlServerDsc.strings.psd1(lines 330–334) and consistently referenced in the public commands:
- RSDR0001:
Remove_SqlDscRole_NotFound(used in Remove-SqlDscRole.ps1)- RSDR0002:
Role_CannotRemoveBuiltIn(used in Remove-SqlDscRole.ps1)- RSDR0003:
Role_RemoveFailed(used in Remove-SqlDscRole.ps1)- ShouldProcess keys correctly defined without trailing periods
- GSDR0001:
Get_SqlDscRole_NotFound(used in Get-SqlDscRole.ps1)All naming conventions followed; no orphaned keys or mismatches detected.
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 7 of 10 files at r1, 5 of 10 files at r3, 5 of 5 files at r4, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @johlju)
1 participant
Pull Request (PR) description
Remove-SqlDscAgentAlert$PSCmdlet.ThrowTerminatingError()instead of exception helperfunctions for proper terminating error handling (issue #2193).
Set-SqlDscAgentAlert$PSCmdlet.ThrowTerminatingError()instead of exception helperfunctions for proper terminating error handling (issue #2196).
Remove-SqlDscDatabase$PSCmdlet.ThrowTerminatingError()instead of exception helperfunctions for proper terminating error handling (issue #2195).
Remove-SqlDscRole$PSCmdlet.ThrowTerminatingError()instead of exception helperfunctions for proper terminating error handling (issue #2199).
New-SqlDscDatabase$PSCmdlet.ThrowTerminatingError()instead of exception helperfunctions for proper terminating error handling (issue #2200).
Set-SqlDscDatabase$PSCmdlet.ThrowTerminatingError()instead of exception helperfunctions for proper terminating error handling (issue #2198).
This Pull Request (PR) fixes the following issues
Remove-SqlDscAgentAlert: Use$PSCmdlet.ThrowTerminatingErrorfor terminating errors #2193Remove-SqlDscDatabase: Use$PSCmdlet.ThrowTerminatingErrorfor terminating errors #2195Set-SqlDscAgentAlert: Use$PSCmdlet.ThrowTerminatingErrorfor terminating errors #2196Set-SqlDscDatabase: Use$PSCmdlet.ThrowTerminatingErrorfor terminating errors #2198Remove-SqlDscRole: Use$PSCmdlet.ThrowTerminatingErrorfor terminating errors #2199New-SqlDscDatabase: Use$PSCmdlet.ThrowTerminatingErrorfor terminating errors #2200Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is