Update module using latest patterns #85
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughRefactors dependency resolution with new bootstrapping options and typed parameters, updates required modules and build/pipeline configurations, removes AppVeyor and Kitchen configs, adds security guidance and changelog note, adjusts VS Code settings, updates module manifest/stubs and localization, and standardizes exception usage in PDT. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as Invoker (script/CI)
participant RD as Resolve-Dependency.ps1
participant CFG as Resolve-Dependency.psd1 (defaults)
participant ENV as PowerShell Env
participant REG as Repository (PSGallery/private)
participant MF as ModuleFast
participant PSRG as PSResourceGet
participant PSGC as PowerShellGetCompat
participant DL as Download/Extract
U->>RD: Call with parameters (UseModuleFast / UsePSResourceGet / UsePowerShellGetCompatibilityModule, RegisterGallery, etc.)
RD->>CFG: Load defaults (conditional)
RD->>ENV: Detect PS version/features
alt RegisterGallery provided
RD->>REG: Register/verify repository
end
alt UseModuleFast branch
RD->>MF: Ensure/Import ModuleFast (version/bleeding-edge)
MF-->>RD: Ready
RD->>MF: Resolve dependencies via ModuleFast
else UsePSResourceGet branch
RD->>PSRG: Ensure PSResourceGet (version/prerelease)
opt PSResourceGet not present
RD->>DL: Download/Zip/Extract PSResourceGet
DL-->>RD: Module path
RD->>PSRG: Import module
end
PSRG-->>RD: Ready
RD->>PSRG: Resolve dependencies via PSResourceGet
else PowerShellGet compatibility branch
RD->>PSGC: Ensure compatibility module / NuGet provider
PSGC-->>RD: Ready
RD->>PSGC: Save/Install modules (Save-Module/Install-Module fallback)
end
RD->>REG: Restore repository state if modified
RD-->>U: Return resolved dependency results (success/failure with logs)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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: 6
🧹 Nitpick comments (1)
source/Modules/PDT/PDT.psm1 (1)
874-874: Consider whetherNew-InvalidOperationExceptionbetter conveys operational failures.These exceptions are raised when process start/stop operations fail after valid arguments are provided.
ArgumentExceptiontypically indicates the argument itself is invalid, whereasInvalidOperationExceptionconveys that an operation failed due to runtime conditions.That said, if the DSC Community's "latest patterns" standardize on
ArgumentExceptionfor all validation-related errors (including deferred validation during execution), the current approach is acceptable.Also applies to: 883-883
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.vscode/settings.json(2 hunks)CHANGELOG.md(1 hunks)RequiredModules.psd1(1 hunks)Resolve-Dependency.ps1(1 hunks)Resolve-Dependency.psd1(1 hunks)SECURITY.md(1 hunks)appveyor.yml(0 hunks)azure-pipelines.yml(1 hunks)build.ps1(6 hunks)build.yaml(5 hunks)kitchen.yml(0 hunks)source/Modules/PDT/PDT.psm1(9 hunks)source/UpdateServicesDsc.psd1(4 hunks)source/UpdateServicesDsc.psm1(1 hunks)source/build.psd1(0 hunks)source/en-US/UpdateServices.strings.psd1(0 hunks)source/en-US/UpdateServicesDsc.strings.psd1(1 hunks)
💤 Files with no reviewable changes (4)
- appveyor.yml
- source/en-US/UpdateServices.strings.psd1
- source/build.psd1
- kitchen.yml
🧰 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- 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/UpdateServicesDsc.psm1azure-pipelines.ymlSECURITY.mdCHANGELOG.mdsource/en-US/UpdateServicesDsc.strings.psd1Resolve-Dependency.psd1build.yamlbuild.ps1source/UpdateServicesDsc.psd1Resolve-Dependency.ps1RequiredModules.psd1source/Modules/PDT/PDT.psm1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
source/UpdateServicesDsc.psm1source/en-US/UpdateServicesDsc.strings.psd1Resolve-Dependency.psd1build.ps1source/UpdateServicesDsc.psd1Resolve-Dependency.ps1RequiredModules.psd1source/Modules/PDT/PDT.psm1
**/*.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:
SECURITY.mdCHANGELOG.md
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
🧠 Learnings (14)
📚 Learning: 2025-10-03T15:27:24.417Z
Learnt from: Borgquite
PR: dsccommunity/UpdateServicesDsc#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/UpdateServicesDsc.psm1source/UpdateServicesDsc.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Import localized strings at the top of the module using Get-LocalizedData
Applied to files:
source/en-US/UpdateServicesDsc.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Store command/function localization in source/en-US/{MyModuleName}.strings.psd1
Applied to files:
source/en-US/UpdateServicesDsc.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Store class localization in source/en-US/{ResourceClassName}.strings.psd1
Applied to files:
source/en-US/UpdateServicesDsc.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/en-US/DSC_*.strings.psd1 : Each resource directory must contain an en-US localization folder with a strings file named DSC_<ResourceName>.strings.psd1
Applied to files:
source/en-US/UpdateServicesDsc.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Define strings using ConvertFrom-StringData with entries like `KeyName = Message with {0} placeholder. (PREFIX0001)`
Applied to files:
source/en-US/UpdateServicesDsc.strings.psd1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
build.yaml
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Applied to files:
build.yaml
📚 Learning: 2025-09-23T10:20:59.832Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-23T10:20:59.832Z
Learning: Applies to build.ps1 : Build the project before running tests by executing: ./build.ps1 -Tasks build
Applied to files:
build.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Build the project before running tests using ./build.ps1 -Tasks build
Applied to files:
build.ps1
📚 Learning: 2025-08-28T15:44:12.628Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2150
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:35-35
Timestamp: 2025-08-28T15:44:12.628Z
Learning: The repository uses RequiredModules.psd1 to specify Pester version requirements with RequiredVersion = 'latest', which ensures compatibility with Should -Invoke syntax (requires Pester 5.4+).
Applied to files:
RequiredModules.psd1
📚 Learning: 2025-08-28T15:44:12.628Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2150
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:35-35
Timestamp: 2025-08-28T15:44:12.628Z
Learning: The SqlServerDsc repository uses RequiredModules.psd1 to specify Pester with Version = 'latest' and AllowPrerelease = $true, ensuring the latest Pester version (well beyond 5.4) is available, which supports Should -Invoke syntax and other modern Pester features.
Applied to files:
RequiredModules.psd1
📚 Learning: 2025-09-28T13:49:31.541Z
Learnt from: dan-hughes
PR: dsccommunity/DhcpServerDsc#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/Modules/PDT/PDT.psm1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/**/*.ps1 : Assume and use $script:localizedData for accessing localized strings
Applied to files:
source/Modules/PDT/PDT.psm1
🪛 markdownlint-cli2 (0.18.1)
SECURITY.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
18-18: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
19-19: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
20-20: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
21-21: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
22-22: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
23-23: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
24-24: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🔇 Additional comments (6)
source/Modules/PDT/PDT.psm1 (1)
6-6: LGTM! Exception type update aligns with latest patterns.The migration from
New-InvalidArgumentExceptiontoNew-ArgumentExceptionis correctly applied across all argument validation scenarios inInvoke-ResolvePathandTest-RootedPath. All calls maintain proper structure with-ArgumentNameand-Messageparameters using localized strings.Also applies to: 37-37, 51-51, 63-63, 72-72, 104-104, 135-135
source/UpdateServicesDsc.psd1 (1)
60-72: Verify that empty export arrays are intentional.All export arrays (FunctionsToExport, CmdletsToExport, VariablesToExport, AliasesToExport, DscResourcesToExport) are set to empty arrays. This is unusual for a DSC module, particularly for
DscResourcesToExport.Based on the AI summary and the new
UpdateServicesDsc.psm1file comment, this appears intentional as the module will be populated during the build process. Please confirm that:
- The build process will correctly populate these exports
- DSC resources will be properly exported after the build
CHANGELOG.md (1)
27-27: Changelog entry looks good.Entry follows Keep a Changelog style and concisely states the change.
Resolve-Dependency.psd1 (1)
2-14: Defaults align with updated dependency flow.The added keys keep Resolve-Dependency consistent with the new PSResourceGet/bootstrap logic.
RequiredModules.psd1 (1)
7-31: Module list refresh looks good.Switching the build/test/doc toolchain to
latestmatches our standard RequiredModules pattern and keeps Pester compatible with modern syntax. Based on learnings..vscode/settings.json (1)
10-66: VS Code settings update LGTM.The added formatting, cSpell, and Pester integration settings align with the refreshed tooling setup.
9 tasks
Contributor
Author
|
@johlju, this needed updating. (I'm surprised it will worked tbh). |
Contributor
johlju
approved these changes
Oct 9, 2025
Member
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 18 of 18 files at r1, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @dan-hughes)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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
Update module to use latest patterns.
This Pull Request (PR) fixes the following issues
Task list
Entry should say what was changed, and how that affects users (if applicable).
and comment-based help.
This change is