xDhcpServer icon indicating copy to clipboard operation
xDhcpServer copied to clipboard

Convert nested modules to buildable modules

Open dan-hughes opened this issue 10 months ago β€’ 3 comments

Pull Request (PR) description

Let ModuleBuilder build DhcpServerDsc.Common and DhcpServerDsc.OptionValueHelper.

This Pull Request (PR) fixes the following issues

Task list

  • [x] 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 added/updated in README.md.
  • [ ] Resource parameter descriptions added/updated in README.md, schema.mof and comment-based help.
  • [ ] Comment-based help added/updated.
  • [ ] Localization strings added/updated in all localization files as appropriate.
  • [ ] Examples appropriately added/updated.
  • [x] Unit tests added/updated. See DSC Community Testing Guidelines.
  • [ ] Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
  • [x] New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

dan-hughes avatar Feb 16 '25 15:02 dan-hughes

@johlju, trying to keep the PR's small.

dan-hughes avatar Feb 16 '25 16:02 dan-hughes

@dan-hughes the HQRM tests (DscResource.Test), for example localization tests, are they run on these private modules? Just curious if I need to be more wary about errors that I could ignored because HQRM would have caught them. πŸ™‚

johlju avatar Feb 16 '25 18:02 johlju

@dan-hughes the HQRM tests (DscResource.Test), for example localization tests, are they run on these private modules? Just curious if I need to be more wary about errors that I could ignored because HQRM would have caught them. πŸ™‚

Looks to be working looking at the logs.

dan-hughes avatar Feb 16 '25 19:02 dan-hughes

Not sure why this is still in draft.

dan-hughes avatar May 28 '25 16:05 dan-hughes

Walkthrough

Restructures DhcpServerDsc.Common and DhcpServerDsc.OptionValueHelper into per-file modules: removes monolithic .psm1 files, adds individual public function files and prefix initialization, updates manifests to export no FunctionsToExport, adjusts build.yaml NestedModule entries, updates CHANGELOG, and replaces aggregated unit tests with per-function tests.

Changes

Cohort / File(s) Summary
Changelog
CHANGELOG.md
Terminates two DhcpServerDsc bullet lines as sentences; notes addition of DhcpServerDsc.Common and DhcpServerDsc.OptionValueHelper as per-file buildable modules.
Build configuration
build.yaml
Removes Modules from CopyPaths; converts NestedModule entries to mapping form; adds NestedModule blocks for DhcpServerDsc.Common and DhcpServerDsc.OptionValueHelper with per-module CopyPaths, Encoding, AddToManifest, and Exclude settings.
DhcpServerDsc.Common β€” manifest & removal
source/Modules/DhcpServerDsc.Common/DhcpServerDsc.Common.psd1, source/Modules/DhcpServerDsc.Common/DhcpServerDsc.Common.psm1
Sets FunctionsToExport to @() in the psd1; removes the legacy monolithic .psm1 implementation.
DhcpServerDsc.Common β€” prefix
source/Modules/DhcpServerDsc.Common/prefix.ps1
Adds prefix that computes and imports DscResource.Common and loads localized data into $script:localizedData.
DhcpServerDsc.Common β€” public functions (new files)
source/Modules/DhcpServerDsc.Common/Public/Assert-ScopeParameter.ps1, .../Get-ValidIPAddress.ps1, .../Get-ValidTimeSpan.ps1, .../New-TerminatingError.ps1, .../Write-PropertyMessage.ps1
Adds per-file public functions: Assert-ScopeParameter, Get-ValidIPAddress, Get-ValidTimeSpan, New-TerminatingError, and Write-PropertyMessage with declared parameters, validation, and terminating-error behavior as described.
DhcpServerDsc.OptionValueHelper β€” manifest & removal
source/Modules/DhcpServerDsc.OptionValueHelper/DhcpServerDsc.OptionValueHelper.psd1, .../DhcpServerDsc.OptionValueHelper.psm1
Sets FunctionsToExport to @() in the psd1; removes the legacy monolithic .psm1 implementation.
DhcpServerDsc.OptionValueHelper β€” prefix
source/Modules/DhcpServerDsc.OptionValueHelper/prefix.ps1
Adds prefix that imports DscResource.Common and loads localized data into $script:localizedData.
DhcpServerDsc.OptionValueHelper β€” public functions (new files)
source/Modules/DhcpServerDsc.OptionValueHelper/Public/Get-TargetResourceHelper.ps1, .../Test-TargetResourceHelper.ps1, .../Set-TargetResourceHelper.ps1
Adds per-file helpers to get, test, and set DHCP option values across Server, Scope, Policy, and ReservedIP targets, with parameter validation, target dispatch, and calls to DHCPServer cmdlets.
Tests β€” DhcpServerDsc.Common (tests removed/added)
tests/Unit/DhcpServerDsc.Common.Tests.ps1, tests/Unit/DhcpServerDsc.Common/Public/*.Tests.ps1
Removes aggregated DhcpServerDsc.Common.Tests.ps1; adds per-function Pester tests for Assert-ScopeParameter, Get-ValidIPAddress, Get-ValidTimeSpan, New-TerminatingError, and Write-PropertyMessage.
Tests β€” DhcpServerDsc.OptionValueHelper (tests removed/added)
tests/Unit/DhcpServerDsc.OptionValueHelper.Tests.ps1, tests/Unit/DhcpServerDsc.OptionValueHelper/Public/*.Tests.ps1
Removes aggregated DhcpServerDsc.OptionValueHelper.Tests.ps1; adds per-function Pester tests for Get-TargetResourceHelper, Test-TargetResourceHelper, and Set-TargetResourceHelper.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant OptionHelper as OptionValueHelper (Public)
  participant DHCP as DHCPServer Cmdlets

  rect rgb(236,248,255)
  note over Caller,OptionHelper: Get-TargetResourceHelper
  Caller->>OptionHelper: Get-TargetResourceHelper(params)
  OptionHelper->>DHCP: Get-DhcpServerv4OptionValue(target-specific)
  DHCP-->>OptionHelper: Option value or null
  OptionHelper-->>Caller: Hashtable { Ensure=Present/Absent, ... }
  end

  rect rgb(240,255,240)
  note over Caller,OptionHelper: Test-TargetResourceHelper
  Caller->>OptionHelper: Test-TargetResourceHelper(params)
  OptionHelper->>OptionHelper: Get-TargetResourceHelper(...)
  OptionHelper-->>Caller: $true / $false
  end

  rect rgb(255,248,236)
  note over Caller,OptionHelper: Set-TargetResourceHelper
  Caller->>OptionHelper: Set-TargetResourceHelper(params, Ensure)
  OptionHelper->>OptionHelper: Get-TargetResourceHelper(...)
  alt Ensure=Present
    OptionHelper->>DHCP: Set-DhcpServerv4OptionValue(target-specific)
  else Ensure=Absent and Present
    OptionHelper->>DHCP: Remove-DhcpServerv4OptionValue(target-specific)
  end
  OptionHelper-->>Caller: (void)
  end
sequenceDiagram
  autonumber
  participant Caller
  participant Common as DhcpServerDsc.Common (Public)
  participant DscCommon as DscResource.Common

  note over Common,DscCommon: prefix.ps1 initialization
  Common->>DscCommon: Import-Module DscResource.Common
  Common->>Common: Get-LocalizedData(en-US)

  rect rgb(246,246,255)
  note over Caller,Common: Assert-ScopeParameter
  Caller->>Common: Assert-ScopeParameter(ScopeId, SubnetMask, Start, End, AF)
  Common->>Common: Get-ValidIPAddress(x4)
  alt invalid range or mismatch
    Common-->>Caller: throw New-TerminatingError(...)
  else valid
    Common-->>Caller: return (no output)
  end
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Pre-merge checks

βœ… Passed checks (3 passed)
Check name Status Explanation
Title Check βœ… Passed The title succinctly captures the primary change of converting the nested modules DhcpServerDsc.Common and DhcpServerDsc.OptionValueHelper into buildable modules, aligning with the core modifications in build.yaml and module restructuring described in the PR. It is concise, focused on the main objective, and void of extraneous details or vague terminology. Scanning the history, a teammate would immediately understand the intent to enable ModuleBuilder support for these previously nested modules.
Description Check βœ… Passed The description clearly states the intent to let ModuleBuilder build the DhcpServerDsc.Common and DhcpServerDsc.OptionValueHelper modules and includes a completed checklist entry for updating the change log and unit tests, matching the changes outlined in the summary. It remains focused on the relevant tasks and ties directly to the code modifications without introducing unrelated content.
Docstring Coverage βœ… Passed No functions found in the changes. Docstring coverage check skipped.

πŸ“œ 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 70ae550b4d6d73d7492f3f2dfd9fe40bd55aaa73 and 75d2e3c305d08a7ded491de0fc157af0b1ad7299.

πŸ“’ Files selected for processing (1)
  • tests/Unit/DhcpServerDsc.Common/Public/Write-PropertyMessage.Tests.ps1 (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/Unit/DhcpServerDsc.Common/Public/Write-PropertyMessage.Tests.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). (7)
  • GitHub Check: dsccommunity.DhcpServerDsc (Test Publish Code Coverage)
  • GitHub Check: dsccommunity.DhcpServerDsc (Test Unit (Windows Server 2025))
  • GitHub Check: dsccommunity.DhcpServerDsc (Test Integration (Windows Server 2022))
  • GitHub Check: dsccommunity.DhcpServerDsc (Test Unit (Windows Server 2022))
  • GitHub Check: dsccommunity.DhcpServerDsc (Test Integration (Windows Server 2025))
  • GitHub Check: dsccommunity.DhcpServerDsc (Test HQRM)
  • GitHub Check: dsccommunity.DhcpServerDsc (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.

coderabbitai[bot] avatar Sep 28 '25 13:09 coderabbitai[bot]