Change verbose messages to debug messages#163
Conversation
WalkthroughSwitched internal diagnostic calls from Write-Verbose to Write-Debug across several public PowerShell functions and updated CHANGELOG.md. No control flow, functional logic, or public API/signature changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@dan-hughes if you have time, can you review this one - should be a simple one |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
CHANGELOG.md (1)
32-44: Reference the tracking issue [#157] for each change entryChangelog items should link to the issue being addressed. Add #157 to these bullets to meet the “Changelog Guidelines”.
Proposed edit (append the issue reference to each first sub-bullet):
- `Compare-DscParameterState` - Changed detailed diagnostic messages from `Write-Verbose` to `Write-Debug` - for variable values, type comparisons, and internal diagnostics. + for variable values, type comparisons, and internal diagnostics ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)). - `Test-ModuleExist` - Changed module filtering messages from `Write-Verbose` to `Write-Debug` - for internal implementation details. + for internal implementation details ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)). - `Test-IsNanoServer` - - Changed operating system SKU diagnostic message from `Write-Verbose` to `Write-Debug`. + - Changed operating system SKU diagnostic message from `Write-Verbose` to `Write-Debug` ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)). - `Find-Certificate` - - Changed certificate filter diagnostic message from `Write-Verbose` to `Write-Debug`. + - Changed certificate filter diagnostic message from `Write-Verbose` to `Write-Debug` ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)). - `Get-LocalizedDataForInvariantCulture` - Changed file processing message from `Write-Verbose` to `Write-Debug` - for internal diagnostic information. + for internal diagnostic information ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)).source/Public/Compare-DscParameterState.ps1 (1)
592-592: Remaining Write-Verbose Calls DetectedThe ripgrep scan confirms only two
Write-Verboseinvocations left insource/Public/Compare-DscParameterState.ps1:
- Line 592:
Write-Verbose -Message $script:localizedData.StartingReverseCheck- Line 633:
Write-Verbose -Message ($localizedString -f $_.Property)If these are intended to provide user-facing, actionable messages, leaving them as
Write-Verboseis perfectly fine. However, if the goal of this PR is to move all diagnostic output toWrite-Debugfor consistency, you may want to update these two calls as well.Optional refactor to convert them to
Write-Debug:--- a/source/Public/Compare-DscParameterState.ps1 +++ b/source/Public/Compare-DscParameterState.ps1 @@ -592,7 +592,7 @@ - Write-Verbose -Message $script:localizedData.StartingReverseCheck + Write-Debug -Message $script:localizedData.StartingReverseCheck @@ -633,7 +633,7 @@ - Write-Verbose -Message ($localizedString -f $_.Property) + Write-Debug -Message ($localizedString -f $_.Property)No other
Write-Verbosecalls were found in this file.source/Public/Find-Certificate.ps1 (1)
179-181: Remove stray -Verbose and backticks; align variable casing to parameter name.
Write-Debugshouldn’t carry-Verbose; it’s a no-op and confusing given the PR intent to move diagnostics to the Debug stream.- Per guidelines, avoid using backticks for line continuation.
- Use
$Store(PascalCase) to match the declared parameter and our naming conventions.Apply this diff:
- Write-Debug ` - -Message ($script:localizedData.SearchingForCertificateUsingFilters -f $store, $certFilterScript) ` - -Verbose + Write-Debug -Message ($script:localizedData.SearchingForCertificateUsingFilters -f $Store, $certFilterScript)
📜 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 (6)
CHANGELOG.md(1 hunks)source/Public/Compare-DscParameterState.ps1(8 hunks)source/Public/Find-Certificate.ps1(1 hunks)source/Public/Get-LocalizedDataForInvariantCulture.ps1(1 hunks)source/Public/Test-IsNanoServer.ps1(1 hunks)source/Public/Test-ModuleExist.ps1(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File 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/Public/Test-ModuleExist.ps1source/Public/Test-IsNanoServer.ps1source/Public/Get-LocalizedDataForInvariantCulture.ps1source/Public/Find-Certificate.ps1CHANGELOG.mdsource/Public/Compare-DscParameterState.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:
@{}- Multi-line: each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- ...
Files:
source/Public/Test-ModuleExist.ps1source/Public/Test-IsNanoServer.ps1source/Public/Get-LocalizedDataForInvariantCulture.ps1source/Public/Find-Certificate.ps1source/Public/Compare-DscParameterState.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/SqlServerDsc.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming Patterns
- Format:
Verb_FunctionName_Action(underscore separators), e.g.Get_SqlDscDatabase_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Public/Test-ModuleExist.ps1source/Public/Test-IsNanoServer.ps1source/Public/Get-LocalizedDataForInvariantCulture.ps1source/Public/Find-Certificate.ps1source/Public/Compare-DscParameterState.ps1
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
🪛 LanguageTool
CHANGELOG.md
[style] ~41-~41: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...Write-Debug. - Find-Certificate - Changed certificate filter diagnostic message f...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~43-~43: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t-LocalizedDataForInvariantCulture - Changed file processing message fromWrite-Ver...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ 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.DscResource.Common (Build Package Module)
🔇 Additional comments (3)
source/Public/Test-IsNanoServer.ps1 (1)
28-28: Approve Write-Debug with LocalizationLocalization key verification succeeded; no further changes required.
- source/Public/Test-IsNanoServer.ps1 (line 28):
Write-Debug -Message ($script:localizedData.TestIsNanoServerOperatingSystemSku -f $operatingSystemSKU)is correctly using the localized string.- source/en-US/DscResource.Common.strings.psd1 (line 4):
TestIsNanoServerOperatingSystemSkuis defined as expected.source/Public/Compare-DscParameterState.ps1 (1)
312-332: Migration to Write-Debug for internal diagnostics looks goodAll the updated message sites now use Write-Debug with localized keys and correct -f formatting. This aligns with the stated goal and the guidelines (debug for diagnostics).
Also applies to: 340-347, 355-356, 371-372, 379-396, 442-514, 540-541, 583-584
source/Public/Find-Certificate.ps1 (1)
179-181: Switch to Write-Debug is aligned with the guidelines.Using Debug for this diagnostic message matches “Use Write-Debug for debugging information” and keeps actionable user-facing info on Verbose separate.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (2)
26-31: Unrelated Get-ComputerName entries appear in this PR’s Unreleased section—please verify scope.These bullets report behavioral changes to
Get-ComputerName, but the PR objective only switches diagnostic output toWrite-Debugfor five other commands. IfGet-ComputerNamewas not modified by this PR, please drop or move these lines to the PR that actually introduces the change to avoid misleading release notes.Proposed minimal fix (remove the unrelated block):
- - `Get-ComputerName` - - Replaced platform-specific logic with cross-platform implementation using - `[System.Environment]::MachineName` for consistent short name behavior. - - Enhanced FQDN functionality to use `[System.Net.Dns]::GetHostByName()` for - proper domain name resolution on Windows, Linux, and macOS. - - Improved error handling to gracefully fallback to short name when DNS - resolution fails.
32-44: Changelog entries match the PR intent; add the issue reference and a short heads-up note.Looks good and aligned with the diagnostic-level switch. Per repo guidelines, include an issue reference and briefly call out the user-facing impact (debug vs. verbose visibility).
Suggested edit (append issue ref to each sub-bullet and add a single-note line):
- `Compare-DscParameterState` - - Changed detailed diagnostic messages from `Write-Verbose` to `Write-Debug` - for variable values, type comparisons, and internal diagnostics. + - Changed detailed diagnostic messages from `Write-Verbose` to `Write-Debug` + for variable values, type comparisons, and internal diagnostics ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)). - `Test-ModuleExist` - - Changed module filtering messages from `Write-Verbose` to `Write-Debug` - for internal implementation details. + - Changed module filtering messages from `Write-Verbose` to `Write-Debug` + for internal implementation details ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)). - `Test-IsNanoServer` - - Changed operating system SKU diagnostic message from `Write-Verbose` to `Write-Debug`. + - Changed operating system SKU diagnostic message from `Write-Verbose` to `Write-Debug` ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)). - `Find-Certificate` - - Changed certificate filter diagnostic message from `Write-Verbose` to `Write-Debug`. + - Changed certificate filter diagnostic message from `Write-Verbose` to `Write-Debug` ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)). - `Get-LocalizedDataForInvariantCulture` - - Changed file processing message from `Write-Verbose` to `Write-Debug` - for internal diagnostic information. + - Changed file processing message from `Write-Verbose` to `Write-Debug` + for internal diagnostic information ([#157](https://github.com/dsccommunity/DscResource.Common/issues/157)). + - Note: Diagnostics moved from verbose to debug now require `-Debug` or `$DebugPreference = 'Continue'` to be displayed.
📜 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)
CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File 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:
CHANGELOG.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
🪛 LanguageTool
CHANGELOG.md
[grammar] ~29-~29: There might be a mistake here.
Context: ...resolution on Windows, Linux, and macOS. - Improved error handling to gracefully fa...
(QB_NEW_EN)
[style] ~41-~41: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...Write-Debug. - Find-Certificate - Changed certificate filter diagnostic message f...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~43-~43: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t-LocalizedDataForInvariantCulture - Changed file processing message fromWrite-Ver...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ 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.DscResource.Common (Build Package Module)
dan-hughes
left a comment
There was a problem hiding this comment.
@dan-hughes reviewed 5 of 6 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @johlju)
source/Public/Compare-DscParameterState.ps1 line 317 at r2 (raw file):
elseif ($currentType.Name -ne 'string') { Write-Debug -Message ($script:localizedData.NoMatchPsCredentialUsernameMessage -f $currentValue.UserName, $desiredValue.UserName)
Should the NoMatch* messages stay Verbose? It then will show in the LCM what is wrong.
I'm thinking of problem, show in Verbose.
source/Public/Compare-DscParameterState.ps1 line 329 at r2 (raw file):
else { Write-Debug -Message ($script:localizedData.NoMatchPsCredentialUsernameMessage -f $currentValue, $desiredValue.UserName)
Same as above.
source/Public/Compare-DscParameterState.ps1 line 340 at r2 (raw file):
$desiredType.FullName -ne $currentType.FullName) { Write-Debug -Message ($script:localizedData.NoMatchTypeMismatchMessage -f $key, $currentType.FullName, $desiredType.FullName)
Same as above.
source/Public/Compare-DscParameterState.ps1 line 346 at r2 (raw file):
elseif ($desiredType.Name -eq 'Unknown' -and $desiredType.Name -ne $currentType.Name) { Write-Debug -Message ($script:localizedData.NoMatchTypeMismatchMessage -f $key, $currentType.Name, $desiredType.Name)
Same as above.
source/Public/Compare-DscParameterState.ps1 line 388 at r2 (raw file):
{ #If only currentvalue is empty, the configuration isn't compliant. Write-Debug -Message ($script:localizedData.NoMatchValueMessage -f $desiredType.FullName, $key, $currentValue, $desiredValue)
Same as above.
source/Public/Compare-DscParameterState.ps1 line 395 at r2 (raw file):
{ #If there is a difference between the number of objects in arrays, this isn't compliant. Write-Debug -Message ($script:localizedData.NoMatchValueDifferentCountMessage -f $desiredType.FullName, $key, $currentValue.Count, $desiredValue.Count)
Same as above.
source/Public/Compare-DscParameterState.ps1 line 442 at r2 (raw file):
$desiredType.FullName -ne $currentType.FullName) { Write-Debug -Message ($script:localizedData.NoMatchElementTypeMismatchMessage -f $key, $i, $currentType.FullName, $desiredType.FullName)
Same as above.
source/Public/Compare-DscParameterState.ps1 line 507 at r2 (raw file):
if ($desiredArrayValues[$i] -ne $currentArrayValues[$i]) { Write-Debug -Message ($script:localizedData.NoMatchElementValueMismatchMessage -f $i, $desiredType.FullName, $key, $currentArrayValues[$i], $desiredArrayValues[$i])
Same as above.
source/Public/Compare-DscParameterState.ps1 line 540 at r2 (raw file):
if ($desiredValue.Keys.Count -eq 0 -and $currentValue.Keys.Count -ne 0) { Write-Debug -Message ($script:localizedData.NoMatchKeyMessage -f $desiredType.FullName, $key, $($currentValue.Keys -join ', '))
Same as above.
source/Public/Compare-DscParameterState.ps1 line 583 at r2 (raw file):
if ($desiredValue -ne $currentValue) { Write-Debug -Message ($script:localizedData.NoMatchValueMessage -f $desiredType.FullName, $key, $currentValue, $desiredValue)
Same as above.
…e from `Write-Verbose` to `Write-Debug` for internal diagnostic information
…rnal details in multiple commands
81f3be7 to
50a4e20
Compare
johlju
left a comment
There was a problem hiding this comment.
Reviewable status: 3 of 6 files reviewed, 10 unresolved discussions (waiting on @dan-hughes)
source/Public/Compare-DscParameterState.ps1 line 317 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Should the NoMatch* messages stay Verbose? It then will show in the LCM what is wrong.
I'm thinking of problem, show in Verbose.
Done. Yes, maybe you right. I kept the InDesiredState and NotInDesiredState verbose, but maybe here we need to show what actually is wrong - easily for the user. Switching back the ones you commented on.
source/Public/Compare-DscParameterState.ps1 line 329 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Same as above.
Done.
source/Public/Compare-DscParameterState.ps1 line 340 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Same as above.
Done.
source/Public/Compare-DscParameterState.ps1 line 346 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Same as above.
Done.
source/Public/Compare-DscParameterState.ps1 line 388 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Same as above.
Done.
source/Public/Compare-DscParameterState.ps1 line 395 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Same as above.
Done.
source/Public/Compare-DscParameterState.ps1 line 442 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Same as above.
Done.
source/Public/Compare-DscParameterState.ps1 line 507 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Same as above.
Done.
source/Public/Compare-DscParameterState.ps1 line 540 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Same as above.
Done.
source/Public/Compare-DscParameterState.ps1 line 583 at r2 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Same as above.
Done.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
source/Public/Test-ModuleExist.ps1 (1)
106-106: Localize Debug messages (duplicate of earlier review).These Write-Debug calls embed hardcoded strings; per localization guidelines they must use $script:localizedData keys with -f formatting.
Apply:
- Write-Debug -Message "Filtering modules by path '$modulePath'." + Write-Debug -Message ($script:localizedData.TestModuleExist_FilteringByPath -f $modulePath) - Write-Debug -Message "Filtering modules by version '$moduleVersion'." + Write-Debug -Message ($script:localizedData.TestModuleExist_FilteringByVersion -f $moduleVersion) - Write-Debug -Message "Filtering modules by prerelease '$modulePrerelease'." + Write-Debug -Message ($script:localizedData.TestModuleExist_FilteringByPrerelease -f $modulePrerelease)Add to source/en-US/*.strings.psd1:
ConvertFrom-StringData @' TestModuleExist_FilteringByPath = Filtering modules by path '{0}'. (TME0001) TestModuleExist_FilteringByVersion = Filtering modules by version '{0}'. (TME0002) TestModuleExist_FilteringByPrerelease = Filtering modules by prerelease '{0}'. (TME0003) '@Also applies to: 122-122, 131-131
📜 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 (6)
CHANGELOG.md(2 hunks)source/Public/Compare-DscParameterState.ps1(5 hunks)source/Public/Find-Certificate.ps1(1 hunks)source/Public/Get-LocalizedDataForInvariantCulture.ps1(1 hunks)source/Public/Test-IsNanoServer.ps1(1 hunks)source/Public/Test-ModuleExist.ps1(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- source/Public/Get-LocalizedDataForInvariantCulture.ps1
- source/Public/Test-IsNanoServer.ps1
🧰 Additional context used
📓 Path-based instructions (5)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File 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/Public/Test-ModuleExist.ps1source/Public/Find-Certificate.ps1CHANGELOG.mdsource/Public/Compare-DscParameterState.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:
@{}- Multi-line: each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- ...
Files:
source/Public/Test-ModuleExist.ps1source/Public/Find-Certificate.ps1source/Public/Compare-DscParameterState.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/SqlServerDsc.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming Patterns
- Format:
Verb_FunctionName_Action(underscore separators), e.g.Get_SqlDscDatabase_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Public/Test-ModuleExist.ps1source/Public/Find-Certificate.ps1source/Public/Compare-DscParameterState.ps1
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
🪛 LanguageTool
CHANGELOG.md
[grammar] ~14-~14: There might be a mistake here.
Context: ...e comparisons, and internal diagnostics. - Comparison result still uses `Write-Verb...
(QB_NEW_EN)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...Write-Debug. - Find-Certificate - Changed certificate filter diagnostic message f...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t-LocalizedDataForInvariantCulture - Changed file processing message fromWrite-Ver...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ 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.DscResource.Common (Build Package Module)
🔇 Additional comments (2)
source/Public/Find-Certificate.ps1 (1)
179-179: LGTM: switched to Write-Debug and removed leftover -Verbose/backticks.source/Public/Compare-DscParameterState.ps1 (1)
312-312: LGTM: non-actionable diagnostics moved to Debug; actionable mismatches remain Verbose.Change aligns with guidelines and preserves localized messages.
Also applies to: 325-325, 355-355, 371-371, 379-379, 383-383, 513-513
…FQDN functionality
|
Always when I switch in short interval between Mac and Windows I manage to hit the wrong key combination when saving and don't realize that I added an 's' 🙂 |
|
Kind of cool that Code Rabbit created issues. But it seems it can only create unit test PR from an existing PR (not tested yet). |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
12-16: Keep result on Write-Verbose: looks good; add issue reference for traceabilityKeeping the user-facing comparison result on Write-Verbose makes sense. Please add a reference to issue #157 to tie this change back to the tracking discussion.
- - Comparison result still uses `Write-Verbose` to provide user-actionable - information about parameter state differences. + - Comparison result still uses `Write-Verbose` to provide user-actionable + information about parameter state differences (see [#157](https://github.com/dsccommunity/DscResource.Common/issues/157)).
📜 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)
CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File 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:
CHANGELOG.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
🪛 LanguageTool
CHANGELOG.md
[grammar] ~14-~14: There might be a mistake here.
Context: ...e comparisons, and internal diagnostics. - Comparison result still uses `Write-Verb...
(QB_NEW_EN)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...Write-Debug. - Find-Certificate - Changed certificate filter diagnostic message f...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t-LocalizedDataForInvariantCulture - Changed file processing message fromWrite-Ver...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ 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.DscResource.Common (Build Package Module)
🔇 Additional comments (4)
CHANGELOG.md (4)
18-19: Test-ModuleExist: change log entry reads clearlyAccurately reflects the switch from Verbose to Debug for internal filtering details.
21-21: Test-IsNanoServer: concise and correctGood clarity on the diagnostic message level change.
23-23: Find-Certificate: entry is preciseClear description of the logging level change.
25-26: Get-LocalizedDataForInvariantCulture: accurate wordingCorrectly scoped to internal diagnostics; no further edits needed.
dan-hughes
left a comment
There was a problem hiding this comment.
Reviewable status: 3 of 6 files reviewed, 10 unresolved discussions (waiting on @johlju)
Pull Request (PR) description
Compare-DscParameterStateWrite-VerbosetoWrite-Debugfor variable values, type comparisons, and internal diagnostics.
Test-ModuleExistWrite-VerbosetoWrite-Debugfor internal implementation details.
Test-IsNanoServerWrite-VerbosetoWrite-Debug.Find-CertificateWrite-VerbosetoWrite-Debug.Get-LocalizedDataForInvariantCultureWrite-VerbosetoWrite-Debugfor internal diagnostic information.
This Pull Request (PR) fixes the following issues
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
DSC Community Testing Guidelines.
This change is