Re-add ReportServerEdition enum to SqlRSSetup class #2126
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: johlju <[email protected]>
Co-authored-by: johlju <[email protected]>
Copilot
AI
changed the title
[WIP] Resolve the issue @dsccommunity/SqlServerDsc/issues/2101
Re-add ReportServerEdition enum to SqlRSSetup class
Aug 3, 2025
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2126 +/- ##
====================================
Coverage 94% 94%
====================================
Files 127 127
Lines 8530 8531 +1
====================================
+ Hits 8077 8078 +1
Misses 453 453
🚀 New features to boost your workflow:
|
|
Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces a new enum ReportServerEdition and updates SqlRSSetup to use it for the Edition property instead of a ValidateSet string. Adjusts Modify parameter construction to include IgnoreZeroEnumValue = $true. Updates unit tests to expect Edition = 0 (unset enum) in specific Get() scenarios. Adds changelog entries. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
johlju
requested changes
Aug 30, 2025
Co-authored-by: johlju <[email protected]>
Member
|
@copilot update changelog according to instructions |
Co-authored-by: johlju <[email protected]>
Contributor
Author
Updated CHANGELOG.md with SqlRSSetup ReportServerEdition enum changes in commit 6f8f653. |
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 (7)
CHANGELOG.md (1)
18-20: Reclassify the changelog entry to match Keep a Changelog types.Re-adding an enum is "Added" and switching a property to an enum is "Changed". Consider moving:
- Added: ReportServerEdition enum
- Changed: SqlRSSetup uses ReportServerEdition for Edition
This keeps "Fixed" for bug fixes only.
source/Enum/001.ReportServerEdition.ps1 (2)
1-6: Tighten the synopsis wording."possible states" → "editions"; "handles" → "handle".
Apply:
- .SYNOPSIS - The possible states for the commands and DSC resources that handles - SQL Server Reporting Services or Power BI Report Server and uses the - parameter Edition. + .SYNOPSIS + Defines the editions used by commands and DSC resources that handle + SQL Server Reporting Services or Power BI Report Server via the + Edition parameter.
7-12: Optional: add an explicit zero/sentinel member for readability.Tests rely on 0 meaning “not set”. A named member (e.g., None = 0) documents that intent without changing behavior.
Example:
enum ReportServerEdition { + None = 0 Developer = 1 Evaluation ExpressAdvanced }tests/Unit/Classes/SqlRSSetup.Tests.ps1 (1)
651-686: Consider adding a unit test to assert Edition is not passed when unset.To lock in IgnoreZeroEnumValue behavior, add a test around Modify() that verifies the constructed command parameters omit Edition when it’s 0.
I can draft a focused Pester It-block that inspects the hashtable from Get-DscProperty and asserts Edition is absent. Do you want that?
source/Classes/020.SqlRSSetup.ps1 (3)
190-191: LGTM: switch Edition to ReportServerEdition.Strong typing here improves validation and downstream parameter building. Consider updating the .PARAMETER Edition help text to list allowed enum values.
295-306: Compare enum to enum to avoid implicit string coercion.$currentEdition.Edition is likely a string; $this.Edition is an enum. Relying on implicit conversions is brittle. Cast the current edition to the enum before comparing.
Apply:
- if ($currentEdition.Edition -ne $this.Edition) + if ([ReportServerEdition] $currentEdition.Edition -ne $this.Edition) { $inDesiredState = $false
624-626: Fix misleading comment.Code normalizes the path; it doesn’t lowercase it.
Apply:
- # Normalize the property to lower case. + # Normalize the path and ensure consistent formatting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)source/Classes/020.SqlRSSetup.ps1(2 hunks)source/Enum/001.ReportServerEdition.ps1(1 hunks)tests/Unit/Classes/SqlRSSetup.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
- Run in PowerShell, from repository root
- Build before running tests:
.\build.ps1 -Tasks build- Always run tests in new PowerShell session:
Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow guidelines over existing code patterns
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/Enum/001.ReportServerEdition.ps1tests/Unit/Classes/SqlRSSetup.Tests.ps1source/Classes/020.SqlRSSetup.ps1CHANGELOG.md
**/*.ps?(m|d)1
⚙️ CodeRabbit configuration file
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: Lis...
Files:
source/Enum/001.ReportServerEdition.ps1tests/Unit/Classes/SqlRSSetup.Tests.ps1source/Classes/020.SqlRSSetup.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/Enum/001.ReportServerEdition.ps1source/Classes/020.SqlRSSetup.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
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- Prefer
-BeTrue/-BeFalseover-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
MockinsideInModuleScope-blockFile Organization
- Class resources:
tests/Unit/Classes/{Name}.Tests.ps1- Public commands:
tests/Unit/Public/{Name}.Tests.ps1- Private functions:
tests/Unit/Private/{Name}.Tests.ps1Data-Driven Tests
- Define variables in separate
BeforeDiscoveryfor-ForEach(close to usage)-ForEachal...
Files:
tests/Unit/Classes/SqlRSSetup.Tests.ps1
source/[cC]lasses/**/*.ps1
⚙️ CodeRabbit configuration file
source/[cC]lasses/**/*.ps1: # DSC Class-Based Resource GuidelinesApplies to: Classes with
[DscResource(...)]decoration only.Requirements
- File:
source/Classes/020.{ResourceName}.ps1- Decoration:
[DscResource(RunAsCredential = 'Optional')](replace with'Mandatory'if required)- Inheritance: Must inherit
ResourceBase(part of module DscResource.Base)$this.localizedDatahashtable auto-populated byResourceBasefrom localization file- value-type properties: Use
[Nullable[{FullTypeName}]](e.g.,[Nullable[System.Int32]])Required constructor
MyResourceName () : base ($PSScriptRoot) { # Property names where state cannot be enforced, e.g. IsSingleInstance, Force $this.ExcludeDscProperties = @() }Required Method Pattern
[MyResourceName] Get() { # Call base implementation to get current state $currentState = ([ResourceBase] $this).Get() # If needed, post-processing on current state that can not be handled by GetCurrentState() return $currentState } [System.Boolean] Test() { # Call base implementation to test current state $inDesiredState = ([ResourceBase] $this).Test() # If needed, post-processing on test result that can not be handled by base Test() return $inDesiredState } [void] Set() { # Call base implementation to set desired state ([ResourceBase] $this).Set() # If needed, additional state changes that can not be handled by Modify() } hidden [System.Collections.Hashtable] GetCurrentState([System.Collections.Hashtable] $properties) { # Always return current state as hashtable, $properties contains key properties } hidden [void] Modify([System.Collections.Hashtable] $properties) { # Always set desired state, $properties contain those that must change state }Optional Method Pattern
hidden [void] AssertProperties([System.Collections.Hashtable] $properties) { # Validate user-provided properties, $p...
Files:
source/Classes/020.SqlRSSetup.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
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
🧠 Learnings (8)
📚 Learning: 2025-08-29T17:24:23.694Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.694Z
Learning: Applies to **/*.{ps1,psm1,psd1} : End files with only one blank line
Applied to files:
source/Enum/001.ReportServerEdition.ps1
📚 Learning: 2025-08-29T17:20:42.238Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.238Z
Learning: Applies to tests/Integration/**/*.ps1 : Use DSCSQLTEST (Database Engine), SSRS (Reporting Services), and PBIRS (Power BI Report Server) instance names in CI integration tests
Applied to files:
tests/Unit/Classes/SqlRSSetup.Tests.ps1
📚 Learning: 2025-08-29T17:21:35.556Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-08-29T17:21:35.556Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Provide hidden Modify([Hashtable] $properties) that applies desired state changes
Applied to files:
source/Classes/020.SqlRSSetup.ps1
📚 Learning: 2025-08-29T17:21:35.556Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-08-29T17:21:35.556Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Optional: hidden NormalizeProperties([Hashtable] $properties) to normalize user-provided properties
Applied to files:
source/Classes/020.SqlRSSetup.ps1
📚 Learning: 2025-08-29T17:21:35.556Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-08-29T17:21:35.556Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Optional: hidden AssertProperties([Hashtable] $properties) to validate user-provided properties
Applied to files:
source/Classes/020.SqlRSSetup.ps1
📚 Learning: 2025-08-29T17:21:35.556Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-08-29T17:21:35.556Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Provide hidden GetCurrentState([Hashtable] $properties) returning a Hashtable of current state
Applied to files:
source/Classes/020.SqlRSSetup.ps1
📚 Learning: 2025-08-28T17:10:34.765Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2150
File: source/Classes/020.SqlAgentAlert.ps1:198-217
Timestamp: 2025-08-28T17:10:34.765Z
Learning: DSC properties with default values (e.g., `$Ensure = 'Present'`) are always present in the properties hashtable passed to validation methods, even when not explicitly specified by the user.
Applied to files:
source/Classes/020.SqlRSSetup.ps1
📚 Learning: 2025-08-29T17:24:23.694Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.694Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Hashtable properties use PascalCase
Applied to files:
source/Classes/020.SqlRSSetup.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). (4)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- 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 (Build Package Module)
🔇 Additional comments (4)
source/Enum/001.ReportServerEdition.ps1 (1)
13-13: LGTM on file termination.File ends with a single trailing blank line per style guideline.
tests/Unit/Classes/SqlRSSetup.Tests.ps1 (2)
132-133: LGTM: assertion updated for enum default (0).The Get() expectation aligns with zero-as-unset enum semantics.
191-192: LGTM: second assertion updated for enum default (0).Consistent with the new Edition handling.
source/Classes/020.SqlRSSetup.ps1 (1)
404-409: LGTM: Include IgnoreZeroEnumValue in Get-DscProperty.This prevents emitting Edition when it’s the default 0 value.
If DscResource.Common is pinned, ensure the version used in the build supports -IgnoreZeroEnumValue on Get-DscProperty.
johlju
approved these changes
Aug 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.
This change is