New-SqlDscDatabase: Add IsLedger parameter#2350
Merged
Conversation
WalkthroughAdds an Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🧰 Additional context used📓 Path-based instructions (5)**⚙️ CodeRabbit configuration file
Files:
tests/[Uu]nit/**/*.[Tt]ests.ps1⚙️ CodeRabbit configuration file
Files:
{**/*.ps1,**/*.psm1,**/*.psd1}⚙️ CodeRabbit configuration file
Files:
**/*.[Tt]ests.ps1⚙️ CodeRabbit configuration file
Files:
source/**/*.ps1⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (17)📚 Learning: 2025-10-26T13:26:13.397ZApplied to files:
📚 Learning: 2025-10-26T13:26:13.396ZApplied to files:
📚 Learning: 2025-10-26T13:26:13.396ZApplied to files:
📚 Learning: 2025-08-17T10:13:30.079ZApplied to files:
📚 Learning: 2025-08-18T13:50:53.789ZApplied to files:
📚 Learning: 2025-10-26T13:27:07.728ZApplied to files:
📚 Learning: 2025-10-26T13:26:13.396ZApplied to files:
📚 Learning: 2025-10-26T13:27:07.728ZApplied to files:
📚 Learning: 2025-10-26T13:27:07.728ZApplied to files:
📚 Learning: 2025-10-12T11:23:30.123ZApplied to files:
📚 Learning: 2025-09-16T16:34:44.689ZApplied to files:
📚 Learning: 2025-09-16T16:34:44.689ZApplied to files:
📚 Learning: 2025-09-16T16:34:44.689ZApplied to files:
📚 Learning: 2025-10-26T13:27:07.728ZApplied to files:
📚 Learning: 2025-09-25T16:38:08.867ZApplied to files:
📚 Learning: 2025-08-16T13:22:15.230ZApplied to files:
📚 Learning: 2025-10-26T13:27:07.728ZApplied to files:
⏰ 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). (5)
🔇 Additional comments (9)
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 |
…Database Co-authored-by: johlju <7189721+johlju@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Move IsLedger parameter to New-SqlDscDatabase
Move IsLedger parameter to New-SqlDscDatabase (creation-time only)
Nov 26, 2025
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1 (1)
365-381: Parameter set expectations correctly updated to dropIsLedgerThe updated
ExpectedParametersstrings forServerObjectSetandDatabaseObjectSetcorrectly remove-IsLedger <bool>, matching the new signature forSet-SqlDscDatabaseProperty. This keeps the parameter set validation aligned with the breaking change that moves ledger handling to creation time.If you want extra protection against regressions, you could also assert that
IsLedgeris not present in$command.Parameters.Keysin the “settable SMO properties” test, but that’s optional. Based on learnings, this follows the existing parameter validation template.- It 'Should have many settable SMO properties available as parameters' { + It 'Should have many settable SMO properties available as parameters' { $command = Get-Command -Name 'Set-SqlDscDatabaseProperty' @@ $command.Parameters.Keys | Should -Contain 'TargetRecoveryTime' + + # Ledger status is immutable post-creation and must not be exposed here + $command.Parameters.Keys | Should -Not -Contain 'IsLedger' }Also applies to: 383-398
source/Public/New-SqlDscDatabase.ps1 (1)
36-42:IsLedgerparameter is correctly scoped and applied during creationThe
IsLedgerparameter is:
- Documented clearly as creation-time only and immutable afterward.
- Scoped only to the
Databaseparameter set (not snapshots).- Applied to the SMO database object only when bound (
$PSBoundParameters.ContainsKey('IsLedger')), beforeCreate()is called.This cleanly implements creation-time ledger support without affecting existing paths.
One improvement to consider: the help text states this requires SQL Server 2022 (version 16) or Azure SQL Database, but the implementation doesn’t currently enforce that. For a clearer failure mode (instead of a generic “create failed”), you could add an explicit capability check similar to
CatalogCollationand throw a targeted error when-IsLedgeris used against unsupported versions/editions. As per coding guidelines, this would better surface version compatibility constraints.Also applies to: 148-150, 327-330
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(2 hunks)source/Public/New-SqlDscDatabase.ps1(3 hunks)source/Public/Set-SqlDscDatabaseProperty.ps1(1 hunks)tests/Unit/Public/New-SqlDscDatabase.Tests.ps1(4 hunks)tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**
⚙️ 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- Classes:
source/Classes/{DependencyGroupNumber}.{ClassName}.ps1- Enums:
source/Enum/{DependencyGroupNumber}.{EnumName}.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-SqlDscDatabaseProperty.ps1source/Public/New-SqlDscDatabase.ps1CHANGELOG.mdtests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.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-SqlDscDatabaseProperty.ps1source/Public/New-SqlDscDatabase.ps1tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.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-SqlDscDatabaseProperty.ps1source/Public/New-SqlDscDatabase.ps1
**/*.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 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
tests/[Uu]nit/**/*.[Tt]ests.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/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
**/*.[Tt]ests.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 or creating an object using an internal class)- 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 ...
Files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
🧠 Learnings (24)
📚 Learning: 2025-09-14T19:17:05.477Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-changelog.instructions.md:0-0
Timestamp: 2025-09-14T19:17:05.477Z
Learning: Applies to CHANGELOG.md : Describe notable changes briefly, with no more than 2 items per change type
Applied to files:
CHANGELOG.md
📚 Learning: 2025-08-16T13:22:15.230Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2134
File: tests/Unit/Public/Get-SqlDscLogin.Tests.ps1:78-93
Timestamp: 2025-08-16T13:22:15.230Z
Learning: In PowerShell unit tests for parameter validation in SqlServerDsc, accessing parameter attributes directly like `$cmd.Parameters['ParameterName'].Attributes.Mandatory` works correctly because PowerShell automatically iterates through the array and returns the property values. Additional filtering to ParameterAttribute is not necessary when the parameter structure is known and controlled.
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.397Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.397Z
Learning: Applies to tests/Integration/**/*.Tests.ps1 : Integration tests must call Disconnect-SqlDscDatabaseEngine after Connect-SqlDscDatabaseEngine
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Applies to tests/Unit/**/*.Tests.ps1 : Unit tests must set $env:SqlServerDscCI = $true in BeforeAll and remove it in AfterAll
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : All public commands must include parameter set validation tests
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Applies to **/*.cs : In Database Engine resources, add InstanceName, ServerName, and Credential to $this.ExcludeDscProperties
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Applies to tests/Integration/**/*.Tests.ps1 : Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to open DB sessions
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : 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
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 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 : Set-TargetResource and Test-TargetResource must have identical parameters
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:28:23.439Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-10-26T13:28:23.439Z
Learning: Applies to **/*.{ps1,psm1} : ValueFromPipeline must be consistent across all parameter set declarations for the same parameter
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Applies to **/*.{ps1,psm1} : Public PowerShell commands must follow the {Verb}-SqlDsc{Noun} naming format
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Use the Parameter Properties template to assert a parameter is mandatory via $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName']; $parameterInfo.Attributes.Mandatory | Should -BeTrue
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:28:23.439Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-10-26T13:28:23.439Z
Learning: Applies to **/*.{ps1,psm1} : Parameters should use full .NET type names
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1
📚 Learning: 2025-08-16T13:35:08.323Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2134
File: source/Public/Get-SqlDscLogin.ps1:0-0
Timestamp: 2025-08-16T13:35:08.323Z
Learning: In PowerShell, users expect to receive $null when no objects are found or when a non-terminating error occurs, rather than empty arrays. This is normal PowerShell behavior and should be maintained in DSC commands.
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1
📚 Learning: 2025-09-25T16:38:08.867Z
Learnt from: johlju
Repo: dsccommunity/ActiveDirectoryDsc PR: 741
File: tests/Integration/MSFT_ADReadOnlyDomainControllerAccount.Integration.Tests.ps1:102-104
Timestamp: 2025-09-25T16:38:08.867Z
Learning: Test-DscConfiguration cmdlet returns string values 'True' or 'False', not boolean values. Therefore, Should -Be 'True' is correct, and Should -BeTrue would be incorrect for this cmdlet.
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Do not add param() inside Pester blocks when using -ForEach
Applied to files:
tests/Unit/Public/Set-SqlDscDatabaseProperty.Tests.ps1
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Applied to files:
tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Applies to tests/Unit/**/*.Tests.ps1 : Unit tests must not mock real SMO types; use SMO stub types from SMO.cs
Applied to files:
tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-08-18T13:50:53.789Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Omit -MockWith when the mock should return $null
Applied to files:
tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Prefix variables used for mocks with 'mock'
Applied to files:
tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never add an empty -MockWith block
Applied to files:
tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Pass all mandatory parameters in tests to avoid prompts
Applied to files:
tests/Unit/Public/New-SqlDscDatabase.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Set $PSDefaultParameterValues entries for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Applied to files:
tests/Unit/Public/New-SqlDscDatabase.Tests.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). (6)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (4)
CHANGELOG.md (2)
25-27: IsLedger removal entry is clear and consistent with existing BREAKING CHANGE blockThe new sub-bullet under the existing
Set-SqlDscDatabasePropertyBREAKING CHANGE correctly documents the removal ofIsLedgerand points users to the proper creation-time command. Wording, nesting, and issue link format all match the established changelog style.
64-66: New-SqlDscDatabase IsLedger addition is well-describedThe added
IsLedgerentry underNew-SqlDscDatabasecleanly explains the new creation-time capability and the read-only nature of ledger status, with a correctly formatted issue reference. This pairs well with the corresponding removal note above.source/Public/Set-SqlDscDatabaseProperty.ps1 (1)
312-315: IsLedger .NOTES entry accurately documents read-only behaviorDocumenting
IsLedgeralongside other read-only-after-creation properties and explicitly pointing users toNew-SqlDscDatabase -IsLedgeraligns the help text with the updated behavior and avoids implying that this command can toggle ledger status.tests/Unit/Public/New-SqlDscDatabase.Tests.ps1 (1)
76-76: Ledger-related tests thoroughly cover behavior and signature
- The SMO stub now includes an
IsLedgernote property, so the unit tests can observe the flag set byNew-SqlDscDatabase.- The new test “Should create a ledger database with IsLedger set to true” verifies that passing
-IsLedger $trueresults in a created object whoseIsLedgerisTrue, which confirms the wiring from parameter to SMO property.- The updated
ExpectedParametersstring for theDatabaseparameter set correctly adds[-IsLedger <bool>]in the expected position.- The dedicated assertion that
IsLedgerhas aParameterSetNameofDatabaseensures the parameter is only available where intended.This is a solid, guideline-compliant extension of the existing test suite. Based on learnings, the parameter set validation and parameter-attribute checks follow the recommended unit test patterns.
Also applies to: 106-112, 169-169, 216-220
Member
|
@copilot the help text states this requires SQL Server 2022 (version 16) or Azure SQL Database, but the implementation doesn’t currently enforce that. For a clearer failure mode (instead of a generic “create failed”), you could add an explicit capability check similar to CatalogCollation and throw a targeted error when -IsLedger is used against unsupported versions/editions. As per coding guidelines, this would better surface version compatibility constraints. |
…22+) Co-authored-by: johlju <7189721+johlju@users.noreply.github.com>
Contributor
Author
Added version validation for |
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Copilot
AI
changed the title
Move IsLedger parameter to New-SqlDscDatabase (creation-time only)
BREAKING CHANGE: Move IsLedger parameter to New-SqlDscDatabase (creation-time only)
Nov 26, 2025
johlju
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2350 +/- ##
====================================
Coverage 94% 94%
====================================
Files 162 162
Lines 9492 9498 +6
====================================
+ Hits 8999 9005 +6
Misses 493 493
🚀 New features to boost your workflow:
|
johlju
approved these changes
Nov 27, 2025
Member
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 6 of 6 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @copilot)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request (PR) description
IsLedgeris read-only after database creation per SQL Server behavior. This moves the parameter fromSet-SqlDscDatabasePropertytoNew-SqlDscDatabasewhere it can actually be set.Changes:
IsLedgerparameter; documented in.NOTESthat ledger status is immutable post-creationIsLedgerparameter to Database parameter set with implementation to set property during creation. Added version validation that throws a clear error (NSD0007) when-IsLedgeris used on SQL Server versions older than 2022 (version 16), following the same pattern asCatalogCollationvalidation.Database_IsLedgerNotSupportederror message stringUsage:
This Pull Request (PR) fixes the following issues
Task 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).
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
This change is