AdcsOnlineResponder: Convert to class-based
#158
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
WalkthroughMigrates AdcsOnlineResponder from a MOF/module-based DSC resource to a class-based DSC resource: removes legacy MOF/module/strings/README and module-based tests, adds Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LCM as "DSC Engine (LCM)"
participant Class as "AdcsOnlineResponder (Class)"
participant ADCS as "ADCSDeployment Module"
User->>LCM: Apply configuration (AdcsOnlineResponder)
LCM->>Class: Invoke Get/Test/Set
Class->>ADCS: Call Install-AdcsOnlineResponder / Uninstall-AdcsOnlineResponder / Install-AdcsOnlineResponder -WhatIf
alt ADCS returns success
ADCS-->>Class: Success
Class-->>LCM: Return state / success
else ADCS throws OnlineResponderSetupException
ADCS-->>Class: OnlineResponderSetupException
Class-->>LCM: Interpret as "Present" / return state
else ADCS throws other error
ADCS-->>Class: Error
Class-->>LCM: Throw localized error (ErrorGetCurrentState / ErrorModifyInstall / ErrorModifyUninstall)
end
LCM-->>User: Report status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
CHANGELOG.md(3 hunks)source/Classes/020.AdcsOnlineResponder.ps1(1 hunks)source/DSCResources/DSC_AdcsOnlineResponder/DSC_AdcsOnlineResponder.psm1(0 hunks)source/DSCResources/DSC_AdcsOnlineResponder/DSC_AdcsOnlineResponder.schema.mof(0 hunks)source/DSCResources/DSC_AdcsOnlineResponder/README.md(0 hunks)source/DSCResources/DSC_AdcsOnlineResponder/en-US/DSC_AdcsOnlineResponder.strings.psd1(0 hunks)source/en-US/AdcsOnlineResponder.strings.psd1(1 hunks)tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1(1 hunks)tests/Unit/DSC_AdcsOnlineResponder.Tests.ps1(0 hunks)
💤 Files with no reviewable changes (5)
- source/DSCResources/DSC_AdcsOnlineResponder/en-US/DSC_AdcsOnlineResponder.strings.psd1
- source/DSCResources/DSC_AdcsOnlineResponder/README.md
- source/DSCResources/DSC_AdcsOnlineResponder/DSC_AdcsOnlineResponder.schema.mof
- tests/Unit/DSC_AdcsOnlineResponder.Tests.ps1
- source/DSCResources/DSC_AdcsOnlineResponder/DSC_AdcsOnlineResponder.psm1
🔇 Additional comments (1)
CHANGELOG.md (1)
10-10: LGTM! Changelog updates are properly formatted and documented.The addition of backticks for code formatting and the new entry documenting the conversion to a class-based resource are appropriate and follow the established changelog format.
Also applies to: 25-25, 38-39
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #158 +/- ##
===================================
- Coverage 98% 98% -1%
===================================
Files 8 10 +2
Lines 514 477 -37
===================================
- Hits 505 468 -37
Misses 9 9 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1 (2)
473-571: Remove skipped tests that reference non-existent properties.These tests reference
AiaUriandOcspUriproperties that don't belong to theAdcsOnlineResponderclass. They appear to be copied fromAdcsAuthorityInformationAccesstests and are not applicable here.
573-854: Remove skipped tests that reference non-existent properties.These
Modify()tests also referenceAiaUriandOcspUriproperties and CA Authority Information Access commands that are not relevant to theAdcsOnlineResponderclass.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
tests/Unit/Classes/AdcsAuthorityInformationAccess.Tests.ps1(1 hunks)tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1(1 hunks)
🔇 Additional comments (1)
tests/Unit/Classes/AdcsAuthorityInformationAccess.Tests.ps1 (1)
597-597: Good catch! Fixed missing closing brace.This corrects the block structure by properly closing the
BeforeAllblock.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1 (4)
304-304: Add missing space before opening brace.-Describe 'AdcsOnlineResponder\Set()' -Tag 'Set' { +Describe 'AdcsOnlineResponder\Set()' -Tag 'Set' {
473-571: Remove skipped tests that reference non-existent properties.These tests reference
AiaUriandOcspUriproperties that don't exist in the AdcsOnlineResponder class. They appear to be copied from another resource and should be removed entirely as they're not applicable to this resource.
573-854: Remove skipped Modify() tests that use incorrect properties.Like the GetCurrentState() tests, these Modify() tests reference non-existent
AiaUriandOcspUriproperties and mock commands that aren't relevant to AdcsOnlineResponder. These tests should be removed or completely rewritten to test the actual functionality of the AdcsOnlineResponder resource.
888-888: Fix incorrect test description.The test description says "Should throw an error" but the test expects it NOT to throw.
- It 'Should throw an error' { + It 'Should not throw an error' {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1(1 hunks)
🔇 Additional comments (3)
tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1 (3)
60-88: Well-structured class instantiation tests.The tests correctly verify that the AdcsOnlineResponder class can be instantiated and has the expected type.
90-302: Comprehensive Get() method test coverage.The tests properly cover all scenarios for the Get() method including desired/not desired states and installed/not installed conditions. Good use of mocking for internal methods.
393-471: Well-implemented Test() method tests.The tests correctly verify the Test() method behavior for both desired and not desired states.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
source/en-US/AdcsOnlineResponder.strings.psd1 (1)
7-13: String resource added and correctly keyed.ErrorGetCurrentState is now active and matches the tests’ expectation. Thanks for closing the loop.
tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1 (1)
574-854: Remove the skipped Modify() tests that reference non-existent AIA/OCSP properties.They’re from another resource and add noise. Either replace with AdcsOnlineResponder-specific Modify() tests or delete.
Apply this diff to remove the irrelevant block:
-Describe 'AdcsOnlineResponder\Modify()' -Tag 'HiddenMember' -Skip:$true { - # ... AIA/OCSP tests ... -}
🧹 Nitpick comments (5)
RequiredModules.psd1 (1)
41-46: Consider pinning to a known-good version to keep CI deterministic.Using 'latest' + prerelease can introduce breaking changes unexpectedly. If stability is preferred, pin a specific version or minimum compatible range once validated.
tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1 (4)
62-68: Avoid “Should -Not -Throw” per test guidelines; this test is redundant.Type/constructor existence is already covered below. Recommend removing this It block.
Apply this diff:
- It 'Should not throw an exception' { - InModuleScope -ScriptBlock { - Set-StrictMode -Version 1.0 - - { [AdcsOnlineResponder]::new() } | Should -Not -Throw - } - }
516-519: Throw a typed exception instead of a string to accurately exercise catch logic.Mocking with a string throws RuntimeException; if the resource catches OnlineResponderSetupException by type, this test may be misleading.
Apply this diff here:
- Mock -CommandName Install-AdcsOnlineResponder -MockWith { - throw 'Microsoft.CertificateServices.Deployment.Common.OCSP.OnlineResponderSetupException' - } + Mock -CommandName Install-AdcsOnlineResponder -MockWith { + throw [Microsoft.CertificateServices.Deployment.Common.OCSP.OnlineResponderSetupException]::new('Already installed') + }And define the mock exception type in AdcsDeploymentStub.psm1:
Add-Type -TypeDefinition @' namespace Microsoft.CertificateServices.Deployment.Common.OCSP { public class OnlineResponderSetupException : System.Exception { public OnlineResponderSetupException() {} public OnlineResponderSetupException(string message) : base(message) {} } } '@
420-425: Return a value from mocked Get() for clarity.Base Test() may read Get()’s output; returning an explicit state makes the tests more robust.
Apply this diff in both Get() mocks:
Add-Member -Force -MemberType 'ScriptMethod' -Name 'Get' -Value { $script:getMethodCallCount += 1 + return @{ + IsSingleInstance = 'Yes' + Ensure = [Ensure]::Present + Reasons = @() + } }Also applies to: 446-449
35-37: Remove commented-out code or implement the mock exception properly.Keep tests clean: either define the exception in the stub (see suggestion above) or delete this commented line.
📜 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)
RequiredModules.psd1(1 hunks)source/Classes/020.AdcsOnlineResponder.ps1(1 hunks)source/en-US/AdcsOnlineResponder.strings.psd1(1 hunks)tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- source/Classes/020.AdcsOnlineResponder.ps1
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run in PowerShell, from repository root
- Build before running tests:
.\build.ps1 -Tasks build- Always run tests in new PowerShell session:
Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow guidelines over existing code patterns
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/en-US/AdcsOnlineResponder.strings.psd1tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1RequiredModules.psd1
**/*.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/en-US/AdcsOnlineResponder.strings.psd1tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1RequiredModules.psd1
**/*.[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/AdcsOnlineResponder.Tests.ps1
🔇 Additional comments (1)
RequiredModules.psd1 (1)
41-46: Allowing prerelease for DscResource.Common looks correct for class-based resources.This aligns with the class-based migration and matches how DscResource.Base is handled. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
tests/Unit/Stubs/AdcsDeploymentStub.psm1 (1)
10-17: Guard Add-Type and add inner-exception ctor for fidelity.Prevents redefinition errors across repeated imports and better mirrors real .NET exceptions.
-Add-Type -TypeDefinition @' +if (-not ('Microsoft.CertificateServices.Deployment.Common.OCSP.OnlineResponderSetupException' -as [type])) +{ +Add-Type -TypeDefinition @' namespace Microsoft.CertificateServices.Deployment.Common.OCSP { public class OnlineResponderSetupException : System.Exception { - public OnlineResponderSetupException() {} - public OnlineResponderSetupException(string message) : base(message) {} + public OnlineResponderSetupException() {} + public OnlineResponderSetupException(string message) : base(message) {} + public OnlineResponderSetupException(string message, System.Exception inner) : base(message, inner) {} } } '@ +}source/Classes/020.AdcsOnlineResponder.ps1 (3)
6-11: Add Required “Requirements” and “Known issues” subsections in DESCRIPTION.Needed by repo guidelines; include link to open issues.
.DESCRIPTION This resource can be used to install an ADCS Online Responder after the feature has been installed on the server. Using this DSC Resource to configure an ADCS Certificate Authority assumes that - the ```ADCS-Online-Responder``` feature has already been installed. + the 'ADCS-Online-Responder' feature has already been installed. + + ## Requirements + - Windows Server with ADCSDeployment module available. + - DscResource.Base and DscResource.Common modules. + + ## Known issues + All issues are not listed here, see all open issues: + https://github.com/dsccommunity/ActiveDirectoryCSDsc/issues?q=is%3Aissue+is%3Aopen+in%3Atitle+AdcsOnlineResponder
140-143: Validate IsSingleInstance value in AssertProperties.Ensure only 'Yes' is accepted; fail fast with localized message.
hidden [void] AssertProperties([System.Collections.Hashtable] $properties) { Assert-Module -ModuleName 'ADCSDeployment' + + if ($properties.ContainsKey('IsSingleInstance') -and $properties.IsSingleInstance -ne 'Yes') + { + # Example key: ErrorIsSingleInstance = "IsSingleInstance must be 'Yes'. Actual: {0} (AOR0003)" + New-InvalidDataException -Message ($this.localizedData.ErrorIsSingleInstance -f $properties.IsSingleInstance) + } }
28-35: Add EXAMPLE section(s) to comment-based help.Required by repo guidelines; include at least Present and Absent examples.
📜 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 (3)
source/Classes/020.AdcsOnlineResponder.ps1(1 hunks)tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1(1 hunks)tests/Unit/Stubs/AdcsDeploymentStub.psm1(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Unit/Classes/AdcsOnlineResponder.Tests.ps1
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ 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:
tests/Unit/Stubs/AdcsDeploymentStub.psm1source/Classes/020.AdcsOnlineResponder.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit configuration file
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: Lis...
Files:
tests/Unit/Stubs/AdcsDeploymentStub.psm1source/Classes/020.AdcsOnlineResponder.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/Classes/020.AdcsOnlineResponder.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.AdcsOnlineResponder.ps1
🧠 Learnings (2)
📓 Common learnings
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryCSDsc#158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.483Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
📚 Learning: 2025-08-30T18:18:31.483Z
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryCSDsc#158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.483Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
Applied to files:
tests/Unit/Stubs/AdcsDeploymentStub.psm1source/Classes/020.AdcsOnlineResponder.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.ActiveDirectoryCSDsc (Test Unit)
- GitHub Check: dsccommunity.ActiveDirectoryCSDsc (Test HQRM)
- GitHub Check: dsccommunity.ActiveDirectoryCSDsc (Test Integration)
- GitHub Check: dsccommunity.ActiveDirectoryCSDsc (Build Package Module)
Comment on lines
104
to
128
| hidden [void] Modify([System.Collections.Hashtable] $properties) | ||
| { | ||
| if ($properties.ContainsKey('Ensure') -and $properties.Ensure -eq [Ensure]::Absent) | ||
| { | ||
| try | ||
| { | ||
| Uninstall-AdcsOnlineResponder -Force | ||
| } | ||
| catch | ||
| { | ||
| New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_ | ||
| } | ||
| } | ||
| else | ||
| { | ||
| try | ||
| { | ||
| Install-AdcsOnlineResponder -Credential $this.Credential -Force | ||
| } | ||
| catch | ||
| { | ||
| New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_ | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Localize error messages and promote non-terminating errors to terminating.
Use -ErrorAction Stop and localized strings with placeholders; avoid leaking raw exception text directly.
hidden [void] Modify([System.Collections.Hashtable] $properties)
{
if ($properties.ContainsKey('Ensure') -and $properties.Ensure -eq [Ensure]::Absent)
{
try
{
- Uninstall-AdcsOnlineResponder -Force
+ Uninstall-AdcsOnlineResponder -Force -ErrorAction Stop
}
catch
{
- New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_
+ # Example key: ErrorModifyUninstall = "Failed to uninstall Online Responder. Details: {0} (AOR0001)"
+ New-InvalidOperationException -Message ($this.localizedData.ErrorModifyUninstall -f $_.Exception.Message) -ErrorRecord $_
}
}
else
{
try
{
- Install-AdcsOnlineResponder -Credential $this.Credential -Force
+ Install-AdcsOnlineResponder -Credential $this.Credential -Force -ErrorAction Stop
}
catch
{
- New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_
+ # Example key: ErrorModifyInstall = "Failed to install Online Responder. Details: {0} (AOR0002)"
+ New-InvalidOperationException -Message ($this.localizedData.ErrorModifyInstall -f $_.Exception.Message) -ErrorRecord $_
}
}
}Add the suggested keys to source/en-US/AdcsOnlineResponder.strings.psd1. I can draft them if helpful.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| hidden [void] Modify([System.Collections.Hashtable] $properties) | |
| { | |
| if ($properties.ContainsKey('Ensure') -and $properties.Ensure -eq [Ensure]::Absent) | |
| { | |
| try | |
| { | |
| Uninstall-AdcsOnlineResponder -Force | |
| } | |
| catch | |
| { | |
| New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_ | |
| } | |
| } | |
| else | |
| { | |
| try | |
| { | |
| Install-AdcsOnlineResponder -Credential $this.Credential -Force | |
| } | |
| catch | |
| { | |
| New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_ | |
| } | |
| } | |
| } | |
| hidden [void] Modify([System.Collections.Hashtable] $properties) | |
| { | |
| if ($properties.ContainsKey('Ensure') -and $properties.Ensure -eq [Ensure]::Absent) | |
| { | |
| try | |
| { | |
| Uninstall-AdcsOnlineResponder -Force -ErrorAction Stop | |
| } | |
| catch | |
| { | |
| # Example key in source/en-US/AdcsOnlineResponder.strings.psd1: | |
| # ErrorModifyUninstall = "Failed to uninstall Online Responder. Details: {0} (AOR0001)" | |
| New-InvalidOperationException -Message ($this.localizedData.ErrorModifyUninstall -f $_.Exception.Message) -ErrorRecord $_ | |
| } | |
| } | |
| else | |
| { | |
| try | |
| { | |
| Install-AdcsOnlineResponder -Credential $this.Credential -Force -ErrorAction Stop | |
| } | |
| catch | |
| { | |
| # Example key in source/en-US/AdcsOnlineResponder.strings.psd1: | |
| # ErrorModifyInstall = "Failed to install Online Responder. Details: {0} (AOR0002)" | |
| New-InvalidOperationException -Message ($this.localizedData.ErrorModifyInstall -f $_.Exception.Message) -ErrorRecord $_ | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In source/Classes/020.AdcsOnlineResponder.ps1 around lines 104-128, the Modify
method currently calls Install-AdcsOnlineResponder/Uninstall-AdcsOnlineResponder
without forcing terminating errors and logs raw exception text; update both
cmdlet calls to include -ErrorAction Stop so non-terminating errors become
terminating, replace direct use of $_.Exception.Message with localized string
lookups containing placeholders (add corresponding keys to
source/en-US/AdcsOnlineResponder.strings.psd1), and in each catch construct and
throw a terminating error using New-InvalidOperationException with the formatted
localized message and the original ErrorRecord so callers receive a proper
terminating error and no raw exception text is leaked.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
source/Classes/020.AdcsOnlineResponder.ps1 (3)
28-29: Use RunAsCredential = 'Optional' to align with module guidelines.Guidelines require 'Optional' (or 'Mandatory' if needed); 'NotSupported' deviates.
-[DscResource(RunAsCredential = 'NotSupported')] +[DscResource(RunAsCredential = 'Optional')] class AdcsOnlineResponder : ResourceBase
95-119: Promote non-terminating errors to terminating and localize error messages in Modify().
- Add -ErrorAction Stop for both cmdlets.
- Use localized messages with placeholders; avoid leaking raw exception text.
hidden [void] Modify([System.Collections.Hashtable] $properties) { if ($properties.ContainsKey('Ensure') -and $properties.Ensure -eq [Ensure]::Absent) { try { - $null = Uninstall-AdcsOnlineResponder -Force + $null = Uninstall-AdcsOnlineResponder -Force -ErrorAction Stop } catch { - New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_ + # Example key: ErrorModifyUninstall = "Failed to uninstall Online Responder. Details: {0} (ADCSOR0002)" + New-InvalidOperationException -Message ($this.localizedData.ErrorModifyUninstall -f $_.Exception.Message) -ErrorRecord $_ } } else { try { - $null = Install-AdcsOnlineResponder -Credential $this.Credential -Force + $null = Install-AdcsOnlineResponder -Credential $this.Credential -Force -ErrorAction Stop } catch { - New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_ + # Example key: ErrorModifyInstall = "Failed to install Online Responder. Details: {0} (ADCSOR0003)" + New-InvalidOperationException -Message ($this.localizedData.ErrorModifyInstall -f $_.Exception.Message) -ErrorRecord $_ } } }If you want, I can draft the en-US string entries (ErrorModifyInstall/Uninstall) and update the unit tests accordingly.
63-82: Make GetCurrentState robust: force terminating errors, avoid type dependency, and do not return from New-InvalidOperationException.
- Add -ErrorAction Stop so errors are catchable.
- Match by message/type text (per learning) rather than CLR type.
- New-* helper throws; don’t return its result.
[System.Collections.Hashtable] GetCurrentState([System.Collections.Hashtable] $properties) { try { - $null = Install-AdcsOnlineResponder -Credential $this.Credential -WhatIf + $null = Install-AdcsOnlineResponder -Credential $this.Credential -WhatIf -ErrorAction Stop return @{} } catch { - if ($_.Exception.GetType().FullName -eq 'Microsoft.CertificateServices.Deployment.Common.OCSP.OnlineResponderSetupException') + if ($_.Exception.ToString() -match 'OnlineResponderSetupException$') { return @{ IsSingleInstance = $this.IsSingleInstance } } - return New-InvalidOperationException -Message $this.localizedData.ErrorGetCurrentState -ErrorRecord $_ + New-InvalidOperationException -Message $this.localizedData.ErrorGetCurrentState -ErrorRecord $_ } }
📜 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 (1)
source/Classes/020.AdcsOnlineResponder.ps1(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ 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/Classes/020.AdcsOnlineResponder.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit configuration file
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: Lis...
Files:
source/Classes/020.AdcsOnlineResponder.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/Classes/020.AdcsOnlineResponder.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.AdcsOnlineResponder.ps1
🧠 Learnings (2)
📓 Common learnings
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryCSDsc#158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.483Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
📚 Learning: 2025-08-30T18:18:31.483Z
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryCSDsc#158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.483Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/Integration/DSC_AdcsCertificationAuthority.Integration.Tests.ps1 (2)
248-251: DRY this by moving AfterEach to the Describe scope.Define a single AfterEach at the Describe 'DSC_AdcsAuthorityInformationAccess_Integration' level to cover both contexts, and remove the two duplicated blocks.
Apply within these lines:
- AfterEach { - Wait-ForIdleLcm - }Add once inside the Describe (outside the contexts), for example right after the config import:
AfterEach { Wait-ForIdleLcm }
297-300: Same here — consolidate AfterEach at Describe level.Remove this duplicate AfterEach if you adopt a single Describe-scoped hook.
Apply within these lines:
- AfterEach { - Wait-ForIdleLcm - }
📜 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 (1)
tests/Integration/DSC_AdcsCertificationAuthority.Integration.Tests.ps1(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ 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:
tests/Integration/DSC_AdcsCertificationAuthority.Integration.Tests.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit configuration file
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: Lis...
Files:
tests/Integration/DSC_AdcsCertificationAuthority.Integration.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
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/Integration/DSC_AdcsCertificationAuthority.Integration.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.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 Stopon 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 build" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' }
Files:
tests/Integration/DSC_AdcsCertificationAuthority.Integration.Tests.ps1
🧠 Learnings (1)
📓 Common learnings
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryCSDsc#158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.483Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
🔇 Additional comments (1)
tests/Integration/DSC_AdcsCertificationAuthority.Integration.Tests.ps1 (1)
248-251: Solid stability improvement: waiting for LCM between tests.This reduces flakiness from overlapping LCM runs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
source/ActiveDirectoryCSDsc.psd1 (2)
30-30: Avoid wildcard export for CmdletsToExport; use empty arrayNo binary cmdlets are present; wildcard hurts manifest hygiene and (slightly) perf. Prefer explicit or empty export.
- CmdletsToExport = '*' + CmdletsToExport = @()
38-40: Explicitly export class-based DSC resourcesList the new AdcsOnlineResponder to aid discovery and keep manifest explicit. Verify the class name matches exactly.
- DscResourcesToExport = @() + DscResourcesToExport = @( + 'AdcsOnlineResponder' + )
📜 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 (2)
RequiredModules.psd1(1 hunks)source/ActiveDirectoryCSDsc.psd1(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- RequiredModules.psd1
🧰 Additional context used
📓 Path-based instructions (2)
**
⚙️ 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/ActiveDirectoryCSDsc.psd1
**/*.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/ActiveDirectoryCSDsc.psd1
🧠 Learnings (1)
📓 Common learnings
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryCSDsc#158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.483Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
Member
|
@dan-hughes is this ready to be reviewed? |
Contributor
Author
|
No, I need to test on a real system. |
Contributor
Author
|
@johlju, good to go. Tested on a real system. |
johlju
reviewed
Jan 12, 2026
Member
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 15 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @dan-hughes).
source/Classes/020.AdcsOnlineResponder.ps1 line 37 at r7 (raw file):
[DscProperty(Mandatory)] [System.Management.Automation.PSCredential] [System.Management.Automation.Credential()]
Do we still need this type, it was just needed for WPS4.0 if I rememeber correctly?
Code quote:
[System.Management.Automation.Credential()]source/Classes/020.AdcsOnlineResponder.ps1 line 68 at r7 (raw file):
try { $null = Install-AdcsOnlineResponder -Credential $this.Credential -WhatIf
Should it be called with WhatIf? If so, can we comment why since it is not normal in prod code. 🤔
Code quote:
$null = Install-AdcsOnlineResponder -Credential $this.Credential -WhatIfsource/Classes/020.AdcsOnlineResponder.ps1 line 105 at r7 (raw file):
catch { New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_
Should we really pass the Message from the same ErrorRecord? Don't we get that from the stacktrac when we pass in ErrorRecord already? Message should be our own localized message instead?
Code quote:
New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_source/Classes/020.AdcsOnlineResponder.ps1 line 116 at r7 (raw file):
catch { New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_
Same as above?
Code quote:
New-InvalidOperationException -Message $_.Exception.Message -ErrorRecord $_
dan-hughes
commented
Jan 13, 2026
Contributor
Author
dan-hughes
left a comment
dan-hughes
left a comment
There was a problem hiding this comment.
@dan-hughes resolved 4 discussions.
Reviewable status: 13 of 15 files reviewed, 1 unresolved discussion (waiting on @johlju).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @source/Classes/020.AdcsOnlineResponder.ps1:
- Around line 62-63: The GetCurrentState([System.Collections.Hashtable]
$properties) method is missing the hidden modifier; update the method
declaration for GetCurrentState in the class to include the hidden keyword
(i.e., make it a hidden method) so it follows the required method pattern;
locate the method named GetCurrentState and prefix its declaration with the
hidden modifier while keeping the return type [System.Collections.Hashtable] and
parameter signature unchanged.
- Around line 72-79: Replace the type equality check in the catch block of
020.AdcsOnlineResponder.ps1 with a string match against the exception text:
instead of comparing $_.Exception.GetType().FullName to
'Microsoft.CertificateServices.Deployment.Common.OCSP.OnlineResponderSetupException',
test whether $_.Exception.ToString() contains a distinguishing substring such as
'OnlineResponderSetupException' (or another reliable token from the exception
text) and return the same @{ IsSingleInstance = $this.IsSingleInstance } when it
matches.
- Around line 6-10: The .DESCRIPTION block in the ADCS Online Responder DSC
resource is missing the required "## Requirements" and "## Known issues"
sections; update the .DESCRIPTION text in
source/Classes/020.AdcsOnlineResponder.ps1 (the ADCS Online Responder DSC class
description) to add a "## Requirements" subsection listing that the target must
be Windows Server with the AD CS role available and that the
ADCS-Online-Responder feature must be installed, and add a "## Known issues"
subsection that points to the open issues link
(https://github.com/dsccommunity/ActiveDirectoryCSDsc/issues?q=is%3Aissue+is%3Aopen+in%3Atitle+AdcsOnlineResponder)
so the resource complies with class-based DSC documentation guidelines.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
source/Classes/020.AdcsOnlineResponder.ps1source/en-US/AdcsOnlineResponder.strings.psd1
🚧 Files skipped from review as they are similar to previous changes (1)
- source/en-US/AdcsOnlineResponder.strings.psd1
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ 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/Classes/020.AdcsOnlineResponder.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 as inline code with a 1‑line description...
Files:
source/Classes/020.AdcsOnlineResponder.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/Classes/020.AdcsOnlineResponder.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.AdcsOnlineResponder.ps1
🧠 Learnings (35)
📓 Common learnings
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2343
File: source/Classes/004.DatabaseFileGroupSpec.ps1:5-34
Timestamp: 2025-11-22T17:36:09.703Z
Learning: The comment-based help requirements for ## Requirements and ## Known issues sections only apply to class-based DSC resources (classes decorated with [DscResource(...)]) in source/Classes/**/*.ps1, not to regular helper classes, DTO classes, or specification classes in the same directory.
Learnt from: dan-hughes
Repo: dsccommunity/ActiveDirectoryCSDsc PR: 158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.528Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
Learnt from: johlju
Repo: dsccommunity/DscResource.Test PR: 167
File: source/Private/Test-FileContainsClassResource.ps1:44-56
Timestamp: 2025-08-09T19:29:36.323Z
Learning: In the DscResource.Test repository, DSC resource attributes are consistently written as `[DscResource(...)]` rather than using variations like `[DscResourceAttribute()]` or fully qualified names like `[Microsoft.PowerShell.DesiredStateConfiguration.DscResource()]`. The Test-FileContainsClassResource function should focus on detecting the standard `[DscResource(...)]` pattern that is actually used in the codebase.
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must inherit from `ResourceBase` class from the DscResource.Base module
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must implement required methods: `Get()`, `Test()`, `Set()`, `GetCurrentState()`, and `Modify()` following the specified pattern
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-NotImplementedException` for not implemented errors instead of `throw` in MOF-based DSC resources
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 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.
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Applies to source/Classes/*.ps1 : DSC resources should always be created as class-based resources
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `try/catch` blocks to handle exceptions in MOF-based DSC resources
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidResultException` for invalid result errors instead of `throw` in MOF-based DSC resources
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ObjectNotFoundException` for object not found errors instead of `throw` in MOF-based DSC resources
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must be decorated with `[DscResource(RunAsCredential = 'Optional')]` or `[DscResource(RunAsCredential = 'Mandatory')]` based on requirements
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-08-30T18:18:31.528Z
Learnt from: dan-hughes
Repo: dsccommunity/ActiveDirectoryCSDsc PR: 158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.528Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must implement a required constructor with signature `ResourceName () : base ($PSScriptRoot)` and populate `$this.ExcludeDscProperties` array
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-22T17:36:09.703Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2343
File: source/Classes/004.DatabaseFileGroupSpec.ps1:5-34
Timestamp: 2025-11-22T17:36:09.703Z
Learning: The comment-based help requirements for ## Requirements and ## Known issues sections only apply to class-based DSC resources (classes decorated with [DscResource(...)]) in source/Classes/**/*.ps1, not to regular helper classes, DTO classes, or specification classes in the same directory.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must implement required methods: `Get()`, `Test()`, `Set()`, `GetCurrentState()`, and `Modify()` following the specified pattern
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T18:00:35.078Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Applies to source/Classes/*.ps1 : DSC resources should always be created as class-based resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must inherit from `ResourceBase` class from the DscResource.Base module
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/020.*.ps1 : DSC class-based resources must use the file naming pattern `source/Classes/020.{ResourceName}.ps1`
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must include comment-based help with .DESCRIPTION section containing Requirements and Known issues sections with a link to all open issues on GitHub
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-10-03T15:27:24.417Z
Learnt from: Borgquite
Repo: dsccommunity/UpdateServicesDsc PR: 78
File: source/DSCResources/MSFT_UpdateServicesComputerTargetGroup/MSFT_UpdateServicesComputerTargetGroup.psm1:9-22
Timestamp: 2025-10-03T15:27:24.417Z
Learning: In the UpdateServicesDsc repository, MOF-based DSC resources follow a minimal comment-based help convention that includes only .SYNOPSIS and .PARAMETER sections. The .DESCRIPTION, .INPUTS, and .OUTPUTS sections are intentionally omitted, even though functions have [OutputType()] attributes. This is consistent across all existing DSC resources: MSFT_UpdateServicesServer, MSFT_UpdateServicesCleanup, and MSFT_UpdateServicesApprovalRule.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `try/catch` blocks to handle exceptions in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : In catch blocks with `Write-Error`, pass original exception using `-Exception` and always use `return` after `Write-Error` to avoid further processing
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : For error handling, use `New-Exception` instead of `[System.Exception]::new(...)`
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `try/catch` blocks for exception handling in classes
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid empty catch blocks (instead use `-ErrorAction SilentlyContinue`)
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `New-*Exception` commands (such as `New-InvalidDataException`, `New-ArgumentException`, `New-InvalidOperationException`, `New-ObjectNotFoundException`, `New-InvalidResultException`, `New-NotImplementedException`) instead of `throw` for terminating errors in classes
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-NotImplementedException` for not implemented errors instead of `throw` in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Place ShouldProcess check immediately before each state-change
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidDataException` for invalid data errors instead of `throw` in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidOperationException` for invalid operation errors instead of `throw` in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidResultException` for invalid result errors instead of `throw` in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `$PSCmdlet.ThrowTerminatingError()` for terminating errors (except for classes), use relevant error category, in try-catch include exception with localized message
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `[Nullable[{FullTypeName}]]` syntax for value-type properties (e.g., `[Nullable[System.Int32]]`)
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Import localized strings using `Get-LocalizedData` at the module top level
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Parameters should use full type name
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use full type name when type casting
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-08-09T19:29:36.323Z
Learnt from: johlju
Repo: dsccommunity/DscResource.Test PR: 167
File: source/Private/Test-FileContainsClassResource.ps1:44-56
Timestamp: 2025-08-09T19:29:36.323Z
Learning: In the DscResource.Test repository, DSC resource attributes are consistently written as `[DscResource(...)]` rather than using variations like `[DscResourceAttribute()]` or fully qualified names like `[Microsoft.PowerShell.DesiredStateConfiguration.DscResource()]`. The Test-FileContainsClassResource function should focus on detecting the standard `[DscResource(...)]` pattern that is actually used in the codebase.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Error` for non-terminating errors with `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID), and `-TargetObject` (object causing error)
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-08-17T10:48:15.384Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:42.327Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-11-27T17:58:42.327Z
Learning: Applies to source/**/*.ps1 : Use localized string keys from $script:localizedData, not hardcoded strings, in diagnostic messages
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use localized strings for all messages including Write-Verbose, Write-Error, and other messaging commands
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:42.327Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-11-27T17:58:42.327Z
Learning: Applies to source/**/*.ps1 : Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages in PowerShell scripts
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-09-28T13:49:31.541Z
Learnt from: dan-hughes
Repo: dsccommunity/DhcpServerDsc PR: 96
File: source/Modules/DhcpServerDsc.Common/Public/Get-ValidTimeSpan.ps1:36-39
Timestamp: 2025-09-28T13:49:31.541Z
Learning: In DSC Community guidelines, ErrorId parameters in New-TerminatingError calls are identifier strings that do not need localization - only the ErrorMessage parameter should use localized strings from $script:localizedData.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'` and restore previous value directly after invocation (do not use try-catch-finally)
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
🔇 Additional comments (7)
source/Classes/020.AdcsOnlineResponder.ps1 (7)
28-45: LGTM!The class declaration properly inherits from
ResourceBase, and properties are correctly defined with full type names. TheRunAsCredential = 'NotSupported'is appropriate since the credential is passed explicitly as a mandatory property.
47-54: LGTM!The constructor follows the required pattern with
base ($PSScriptRoot)and correctly excludesIsSingleInstanceandCredentialfrom state enforcement.
56-60: LGTM!The
Get()method correctly delegates to the base implementation.
85-89: LGTM!The
Set()method correctly delegates to the base implementation.
91-119: LGTM!The
Modify()method is properly implemented with:
- Correct
hiddenmodifier- Appropriate handling for both install and uninstall scenarios
- Proper error handling using
New-InvalidOperationExceptionwith localized messages
121-125: LGTM!The
Test()method correctly delegates to the base implementation.
127-135: LGTM!The
AssertProperties()method appropriately validates that theADCSDeploymentmodule is available before attempting operations.
Comment on lines
+72
to
+79
| catch | ||
| { | ||
| if ($_.Exception.GetType().FullName -eq 'Microsoft.CertificateServices.Deployment.Common.OCSP.OnlineResponderSetupException') | ||
| { | ||
| return @{ | ||
| IsSingleInstance = $this.IsSingleInstance | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use string matching on exception message instead of type comparison.
Based on learnings, for ADCS resources use string matching on $_.Exception.ToString() instead of checking .GetType().FullName. This avoids dependency on types that may not be available in CI systems or DSC authoring machines.
📝 Suggested fix
catch
{
- if ($_.Exception.GetType().FullName -eq 'Microsoft.CertificateServices.Deployment.Common.OCSP.OnlineResponderSetupException')
+ if ($_.Exception.ToString() -match 'OnlineResponderSetupException$')
{
return @{
IsSingleInstance = $this.IsSingleInstance
}
}🤖 Prompt for AI Agents
In @source/Classes/020.AdcsOnlineResponder.ps1 around lines 72 - 79, Replace the
type equality check in the catch block of 020.AdcsOnlineResponder.ps1 with a
string match against the exception text: instead of comparing
$_.Exception.GetType().FullName to
'Microsoft.CertificateServices.Deployment.Common.OCSP.OnlineResponderSetupException',
test whether $_.Exception.ToString() contains a distinguishing substring such as
'OnlineResponderSetupException' (or another reliable token from the exception
text) and return the same @{ IsSingleInstance = $this.IsSingleInstance } when it
matches.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @source/Classes/020.AdcsOnlineResponder.ps1:
- Around line 6-11: Update the .DESCRIPTION block in the ADCS Online Responder
DSC resource (class/file: 020.AdcsOnlineResponder.ps1) to include a ##
Requirements section and a ## Known issues section; add a brief Requirements
bullet stating the target must be Windows Server with the ADCS-Online-Responder
feature installed, and add a Known issues entry that points to the repository’s
open issues for AdcsOnlineResponder (use the provided URL
https://github.com/dsccommunity/ActiveDirectoryCSDsc/issues?q=is%3Aissue+is%3Aopen+in%3Atitle+AdcsOnlineResponder);
ensure the new sections follow the existing .DESCRIPTION text and use the same
indentation/formatting style as the surrounding comments.
🧹 Nitpick comments (1)
source/Classes/020.AdcsOnlineResponder.ps1 (1)
62-84: Consider returningEnsurein the current state hashtable.When the Online Responder is detected as installed (exception caught), the returned state only includes
IsSingleInstance. Typically, DSC resources returnEnsure = 'Present'to indicate the current installation state, which helps the base class properly compare desired vs. current state.📝 Suggested change
if ($_.Exception.GetType().FullName -eq 'Microsoft.CertificateServices.Deployment.Common.OCSP.OnlineResponderSetupException') { return @{ IsSingleInstance = $this.IsSingleInstance + Ensure = [Ensure]::Present } }Please verify whether the
ResourceBaseclass expectsEnsurein the returned state for proper comparison, or if it handles this automatically.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/Classes/020.AdcsOnlineResponder.ps1
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ 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/Classes/020.AdcsOnlineResponder.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 as inline code with a 1‑line description...
Files:
source/Classes/020.AdcsOnlineResponder.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/Classes/020.AdcsOnlineResponder.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.AdcsOnlineResponder.ps1
🧠 Learnings (38)
📓 Common learnings
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2343
File: source/Classes/004.DatabaseFileGroupSpec.ps1:5-34
Timestamp: 2025-11-22T17:36:09.703Z
Learning: The comment-based help requirements for ## Requirements and ## Known issues sections only apply to class-based DSC resources (classes decorated with [DscResource(...)]) in source/Classes/**/*.ps1, not to regular helper classes, DTO classes, or specification classes in the same directory.
Learnt from: dan-hughes
Repo: dsccommunity/ActiveDirectoryCSDsc PR: 158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.528Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
Learnt from: johlju
Repo: dsccommunity/DscResource.Test PR: 167
File: source/Private/Test-FileContainsClassResource.ps1:44-56
Timestamp: 2025-08-09T19:29:36.323Z
Learning: In the DscResource.Test repository, DSC resource attributes are consistently written as `[DscResource(...)]` rather than using variations like `[DscResourceAttribute()]` or fully qualified names like `[Microsoft.PowerShell.DesiredStateConfiguration.DscResource()]`. The Test-FileContainsClassResource function should focus on detecting the standard `[DscResource(...)]` pattern that is actually used in the codebase.
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must inherit from `ResourceBase` class from the DscResource.Base module
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must implement required methods: `Get()`, `Test()`, `Set()`, `GetCurrentState()`, and `Modify()` following the specified pattern
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-NotImplementedException` for not implemented errors instead of `throw` in MOF-based DSC resources
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 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.
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Applies to source/Classes/*.ps1 : DSC resources should always be created as class-based resources
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `try/catch` blocks to handle exceptions in MOF-based DSC resources
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidResultException` for invalid result errors instead of `throw` in MOF-based DSC resources
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ObjectNotFoundException` for object not found errors instead of `throw` in MOF-based DSC resources
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must be decorated with `[DscResource(RunAsCredential = 'Optional')]` or `[DscResource(RunAsCredential = 'Mandatory')]` based on requirements
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-08-30T18:18:31.528Z
Learnt from: dan-hughes
Repo: dsccommunity/ActiveDirectoryCSDsc PR: 158
File: source/Classes/020.AdcsOnlineResponder.ps1:0-0
Timestamp: 2025-08-30T18:18:31.528Z
Learning: For ADCS resources in ActiveDirectoryCSDsc, when catching exceptions from Install-Adcs* commands to determine installation state, use string matching on the exception message (e.g., `$_.Exception.ToString() -match 'OnlineResponderSetupException$'`) instead of catching specific exception types. This avoids dependency on importing types that may not be available in all environments like CI systems or DSC authoring machines.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-22T17:36:09.703Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2343
File: source/Classes/004.DatabaseFileGroupSpec.ps1:5-34
Timestamp: 2025-11-22T17:36:09.703Z
Learning: The comment-based help requirements for ## Requirements and ## Known issues sections only apply to class-based DSC resources (classes decorated with [DscResource(...)]) in source/Classes/**/*.ps1, not to regular helper classes, DTO classes, or specification classes in the same directory.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must implement a required constructor with signature `ResourceName () : base ($PSScriptRoot)` and populate `$this.ExcludeDscProperties` array
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must implement required methods: `Get()`, `Test()`, `Set()`, `GetCurrentState()`, and `Modify()` following the specified pattern
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must include comment-based help with .DESCRIPTION section containing Requirements and Known issues sections with a link to all open issues on GitHub
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T18:00:35.078Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Applies to source/Classes/*.ps1 : DSC resources should always be created as class-based resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must inherit from `ResourceBase` class from the DscResource.Base module
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-10-03T15:27:24.417Z
Learnt from: Borgquite
Repo: dsccommunity/UpdateServicesDsc PR: 78
File: source/DSCResources/MSFT_UpdateServicesComputerTargetGroup/MSFT_UpdateServicesComputerTargetGroup.psm1:9-22
Timestamp: 2025-10-03T15:27:24.417Z
Learning: In the UpdateServicesDsc repository, MOF-based DSC resources follow a minimal comment-based help convention that includes only .SYNOPSIS and .PARAMETER sections. The .DESCRIPTION, .INPUTS, and .OUTPUTS sections are intentionally omitted, even though functions have [OutputType()] attributes. This is consistent across all existing DSC resources: MSFT_UpdateServicesServer, MSFT_UpdateServicesCleanup, and MSFT_UpdateServicesApprovalRule.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/020.*.ps1 : DSC class-based resources must use the file naming pattern `source/Classes/020.{ResourceName}.ps1`
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `try/catch` blocks to handle exceptions in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : In catch blocks with `Write-Error`, pass original exception using `-Exception` and always use `return` after `Write-Error` to avoid further processing
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : For error handling, use `New-Exception` instead of `[System.Exception]::new(...)`
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `try/catch` blocks for exception handling in classes
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid empty catch blocks (instead use `-ErrorAction SilentlyContinue`)
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `New-*Exception` commands (such as `New-InvalidDataException`, `New-ArgumentException`, `New-InvalidOperationException`, `New-ObjectNotFoundException`, `New-InvalidResultException`, `New-NotImplementedException`) instead of `throw` for terminating errors in classes
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-NotImplementedException` for not implemented errors instead of `throw` in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Place ShouldProcess check immediately before each state-change
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidDataException` for invalid data errors instead of `throw` in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidOperationException` for invalid operation errors instead of `throw` in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidResultException` for invalid result errors instead of `throw` in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `$PSCmdlet.ThrowTerminatingError()` for terminating errors (except for classes), use relevant error category, in try-catch include exception with localized message
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `[Nullable[{FullTypeName}]]` syntax for value-type properties (e.g., `[Nullable[System.Int32]]`)
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Import localized strings using `Get-LocalizedData` at the module top level
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Parameters should use full type name
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use full type name when type casting
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-08-09T19:29:36.323Z
Learnt from: johlju
Repo: dsccommunity/DscResource.Test PR: 167
File: source/Private/Test-FileContainsClassResource.ps1:44-56
Timestamp: 2025-08-09T19:29:36.323Z
Learning: In the DscResource.Test repository, DSC resource attributes are consistently written as `[DscResource(...)]` rather than using variations like `[DscResourceAttribute()]` or fully qualified names like `[Microsoft.PowerShell.DesiredStateConfiguration.DscResource()]`. The Test-FileContainsClassResource function should focus on detecting the standard `[DscResource(...)]` pattern that is actually used in the codebase.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Error` for non-terminating errors with `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID), and `-TargetObject` (object causing error)
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-08-17T10:48:15.384Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:42.327Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-11-27T17:58:42.327Z
Learning: Applies to source/**/*.ps1 : Use localized string keys from $script:localizedData, not hardcoded strings, in diagnostic messages
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use localized strings for all messages including Write-Verbose, Write-Error, and other messaging commands
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:58:42.327Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-11-27T17:58:42.327Z
Learning: Applies to source/**/*.ps1 : Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages in PowerShell scripts
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-12-24T18:09:36.332Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-24T18:09:36.332Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'` and restore previous value directly after invocation (do not use try-catch-finally)
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-09-28T13:49:31.541Z
Learnt from: dan-hughes
Repo: dsccommunity/DhcpServerDsc PR: 96
File: source/Modules/DhcpServerDsc.Common/Public/Get-ValidTimeSpan.ps1:36-39
Timestamp: 2025-09-28T13:49:31.541Z
Learning: In DSC Community guidelines, ErrorId parameters in New-TerminatingError calls are identifier strings that do not need localization - only the ErrorMessage parameter should use localized strings from $script:localizedData.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Every DSC resource must define the three required functions: `Get-TargetResource`, `Set-TargetResource`, and `Test-TargetResource`
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-10-03T15:27:39.511Z
Learnt from: Borgquite
Repo: dsccommunity/UpdateServicesDsc PR: 78
File: source/DSCResources/MSFT_UpdateServicesComputerTargetGroup/MSFT_UpdateServicesComputerTargetGroup.psm1:91-109
Timestamp: 2025-10-03T15:27:39.511Z
Learning: For MOF-based DSC resources in DSC Community repositories (files matching `source/DSCResources/MSFT_*/MSFT_*.psm1`), the comment-based help for `Get-TargetResource`, `Set-TargetResource`, and `Test-TargetResource` functions should NOT include `.OUTPUTS` sections, even though the general PowerShell guidelines require them. This is an established pattern in the DSC Community.
Applied to files:
source/Classes/020.AdcsOnlineResponder.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ObjectNotFoundException` for object not found errors instead of `throw` in MOF-based DSC resources
Applied to files:
source/Classes/020.AdcsOnlineResponder.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). (1)
- GitHub Check: dsccommunity.ActiveDirectoryCSDsc (Build Package Module)
🔇 Additional comments (6)
source/Classes/020.AdcsOnlineResponder.ps1 (6)
28-54: LGTM!The class declaration correctly inherits from
ResourceBase, properties are properly decorated, and the constructor follows the required pattern withbase($PSScriptRoot)and populatesExcludeDscProperties.
56-60: LGTM!The
Get()method correctly delegates to the base implementation.
86-90: LGTM!The
Set()method correctly delegates to the base implementation.
96-120: LGTM!The
Modify()method implements proper error handling usingNew-InvalidOperationExceptionwith localized messages and passes the original error record. The install/uninstall logic is clear and follows the expected pattern.
122-126: LGTM!The
Test()method correctly delegates to the base implementation.
128-136: LGTM!The
AssertProperties()method correctly validates the presence of the requiredADCSDeploymentmodule before operations.
Comment on lines
+6
to
+11
| .DESCRIPTION | ||
| This resource can be used to install an ADCS Online Responder after the feature | ||
| has been installed on the server. | ||
| Using this DSC Resource to configure an ADCS Online Responder assumes that | ||
| the ```ADCS-Online-Responder``` feature has already been installed. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add required ## Requirements and ## Known issues sections to .DESCRIPTION.
Per coding guidelines, DSC class-based resources must include ## Requirements and ## Known issues sections in the .DESCRIPTION with a link to all open issues.
📝 Suggested addition to .DESCRIPTION
.DESCRIPTION
This resource can be used to install an ADCS Online Responder after the feature
has been installed on the server.
Using this DSC Resource to configure an ADCS Online Responder assumes that
the ```ADCS-Online-Responder``` feature has already been installed.
+
+ ## Requirements
+
+ * Target machine must be running Windows Server with the ADCS-Online-Responder
+ feature installed.
+
+ ## Known issues
+
+ All issues are not listed here, see [all open issues](https://github.com/dsccommunity/ActiveDirectoryCSDsc/issues?q=is%3Aissue+is%3Aopen+in%3Atitle+AdcsOnlineResponder).🤖 Prompt for AI Agents
In @source/Classes/020.AdcsOnlineResponder.ps1 around lines 6 - 11, Update the
.DESCRIPTION block in the ADCS Online Responder DSC resource (class/file:
020.AdcsOnlineResponder.ps1) to include a ## Requirements section and a ## Known
issues section; add a brief Requirements bullet stating the target must be
Windows Server with the ADCS-Online-Responder feature installed, and add a Known
issues entry that points to the repository’s open issues for AdcsOnlineResponder
(use the provided URL
https://github.com/dsccommunity/ActiveDirectoryCSDsc/issues?q=is%3Aissue+is%3Aopen+in%3Atitle+AdcsOnlineResponder);
ensure the new sections follow the existing .DESCRIPTION text and use the same
indentation/formatting style as the surrounding comments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Convert AdcsOnlineResponder to class-based resource
This Pull Request (PR) fixes the following issues
AdcsOnlineResponder: Convert to class-based resource #155Task list
Entry should say what was changed, and how that affects users (if applicable).
and comment-based help.
This change is