Skip to content

New-SqlDscRole: Fix verbose output#2359

Merged
johlju merged 2 commits intodsccommunity:mainfrom
johlju:fix/issue-#2156
Dec 5, 2025
Merged

New-SqlDscRole: Fix verbose output#2359
johlju merged 2 commits intodsccommunity:mainfrom
johlju:fix/issue-#2156

Conversation

@johlju
Copy link
Copy Markdown
Member

@johlju johlju commented Dec 4, 2025

Pull Request (PR) description

  • New-SqlDscRole
    • Fixed duplicate verbose output by removing manual Write-Verbose call, as
      PSCmdlet.ShouldProcess already generates appropriate verbose output
      (issue #2156).

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Resource documentation updated in the resource's README.md.
  • Resource parameter descriptions updated in schema.mof.
  • Comment-based help updated, including parameter descriptions.
  • Localization strings updated.
  • Examples updated.
  • Unit tests updated. See DSC Community Testing Guidelines.
  • Integration tests updated (where possible). See DSC Community Testing Guidelines.
  • Code changes adheres to DSC Community Style Guidelines.

This change is Reviewable

@johlju johlju requested a review from a team as a code owner December 4, 2025 17:13
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 4, 2025

Walkthrough

Added a new public cmdlet New-SqlDscRole and removed a redundant manual Write-Verbose call and its corresponding resource string, leaving verbose output to PSCmdlet.ShouldProcess. No public signatures were changed.

Changes

Cohort / File(s) Summary
Cmdlet implementation
source/Public/New-SqlDscRole.ps1
Added the New-SqlDscRole(...) public cmdlet implementation and removed a manual Write-Verbose call that emitted the "Creating server role..." message.
Localization strings
source/en-US/SqlServerDsc.strings.psd1
Removed the Role_Create resource string entry used by the deleted Write-Verbose message; other related strings remain.
Changelog
CHANGELOG.md
Added entry announcing the new New-SqlDscRole command and the fix for duplicate verbose output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review source/Public/New-SqlDscRole.ps1 to confirm the new cmdlet is properly exported and that ShouldProcess usage provides expected messaging and behavior.
  • Verify source/en-US/SqlServerDsc.strings.psd1 removal doesn't leave unresolved references.
  • Quick sanity-check of CHANGELOG.md entry for clarity and accuracy.

Pre-merge checks

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing duplicate verbose output in the New-SqlDscRole command by removing manual Write-Verbose calls.
Description check ✅ Passed The description is directly related to the changeset, explaining the fix for duplicate verbose output and referencing the linked issue #2156.
Linked Issues check ✅ Passed The pull request successfully addresses issue #2156 by removing the duplicate manual Write-Verbose call in New-SqlDscRole, relying instead on PSCmdlet.ShouldProcess for verbose output.
Out of Scope Changes check ✅ Passed All changes are in scope: removing the duplicate Write-Verbose call, removing the associated localization string Role_Create, and updating CHANGELOG.md—all directly address the issue of eliminating redundant verbose output.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db26e3a and d4a497d.

📒 Files selected for processing (1)
  • CHANGELOG.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
  • GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
  • GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
  • GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78ece95 and db26e3a.

📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • source/Public/New-SqlDscRole.ps1 (0 hunks)
  • source/en-US/SqlServerDsc.strings.psd1 (0 hunks)
💤 Files with no reviewable changes (2)
  • source/Public/New-SqlDscRole.ps1
  • source/en-US/SqlServerDsc.strings.psd1
🧰 Additional context used
📓 Path-based instructions (3)
CHANGELOG.md

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)

CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format in CHANGELOG.md
Describe notable changes briefly in CHANGELOG.md, ≤2 items per change type
Reference issues using format issue #<issue_number> in CHANGELOG.md
No empty lines between list items in same section of CHANGELOG.md
Skip adding entry if same change already exists in Unreleased section of CHANGELOG.md
No duplicate sections or items in Unreleased section of CHANGELOG.md

Always update CHANGELOG.md Unreleased section

Files:

  • CHANGELOG.md

⚙️ CodeRabbit configuration file

CHANGELOG.md: # Changelog Guidelines

  • Always update the Unreleased section in CHANGELOG.md
  • Use Keep a Changelog format
  • Describe notable changes briefly, ≤2 items per change type
  • Reference issues using format issue #<issue_number>
  • No empty lines between list items in same section
  • Skip adding entry if same change already exists in Unreleased section
  • No duplicate sections or items in Unreleased section

Files:

  • CHANGELOG.md
**/*.md

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)

**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown files
Use '1.' for all items in ordered lists (1/1/1 numbering style)
Disable MD013 rule 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
Format parameters using bold in Markdown documentation
Format values and literals using inline code in Markdown documentation
Format resource/module/product names using italic in Markdown documentation
Format commands/files/paths using inline code in Markdown documentation

Files:

  • CHANGELOG.md

⚙️ CodeRabbit configuration file

**/*.md: # Markdown Style Guidelines

  • Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
  • Use 2 spaces for indentation
  • Use '1.' for all items in ordered lists (1/1/1 numbering style)
  • Disable MD013 rule 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
**

⚙️ CodeRabbit configuration file

**: # DSC Community Guidelines

Terminology

  • 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 pwsh session): ./build.ps1 -Tasks noop
  • Build project before running tests: ./build.ps1 -Tasks build
  • Always run tests in new pwsh session: Invoke-Pester -Path @({test paths}) -Output Detailed

File 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.ps1

Requirements

  • 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:

  • CHANGELOG.md
🧠 Learnings (4)
📓 Common learnings
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.
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 0
File: :0-0
Timestamp: 2025-09-06T19:18:18.857Z
Learning: When a PowerShell command uses PSCmdlet.ShouldProcess and is called with -Verbose, the description message from ShouldProcess is automatically output as a verbose message. Therefore, there is no need to use Write-Verbose before or inside PSCmdlet.ShouldProcess blocks as PSCmdlet.ShouldProcess already provides the required verbose message through its description parameter.
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 0
File: :0-0
Timestamp: 2025-09-06T19:18:18.857Z
Learning: When a PowerShell command uses PSCmdlet.ShouldProcess and is called with -Verbose, the description message from ShouldProcess is automatically output as a verbose message. Therefore, there is no need to use Write-Verbose before or inside PSCmdlet.ShouldProcess blocks as PSCmdlet.ShouldProcess already provides the required verbose message through its description parameter.
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
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.169Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid using `Write-Verbose` inside `$PSCmdlet.ShouldProcess`-block
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.169Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Verbose` for high-level execution flow and user-actionable information only
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.169Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid `Write-Host` cmdlet; use `Write-Verbose`, `Write-Information`, or other output streams instead
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 : Each DSC resource function must include at least one `Write-Verbose` statement
Learnt from: dan-hughes
Repo: dsccommunity/DscResource.Base PR: 48
File: source/Classes/010.ResourceBase.ps1:150-156
Timestamp: 2025-09-28T13:22:40.783Z
Learning: Class-based DSC resources (source/[cC]lasses/**/*.ps1) do not require verbose banners in Set() method like MOF-based resources do. The focus should be on relevant state information only.
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.169Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Do not redefine reserved parameters such as Verbose and Debug
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/*.ps1 : Format public commands as `{Verb}-SqlDsc{Noun}` following PowerShell naming conventions

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`

Applied to files:

  • CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
  • GitHub Check: PSScriptAnalyzer
  • GitHub Check: PSScriptAnalyzer

Comment thread CHANGELOG.md
Copy link
Copy Markdown
Member Author

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@johlju reviewed 2 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @johlju)

@johlju
Copy link
Copy Markdown
Member Author

johlju commented Dec 4, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 4, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94%. Comparing base (78ece95) to head (d4a497d).
⚠️ Report is 32 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@         Coverage Diff          @@
##           main   #2359   +/-   ##
====================================
- Coverage    94%     94%   -1%     
====================================
  Files       166     166           
  Lines      9765    9764    -1     
====================================
- Hits       9206    9205    -1     
  Misses      559     559           
Flag Coverage Δ
unit 94% <ø> (-1%) ⬇️
Files with missing lines Coverage Δ
source/Public/New-SqlDscRole.ps1 100% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@johlju johlju merged commit 918557f into dsccommunity:main Dec 5, 2025
31 checks passed
@johlju johlju deleted the fix/issue-#2156 branch December 5, 2025 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New-SqlDscRole: Unnecessary verbose output

1 participant