[ResourceBase]: Add Microsoft DSC support - #54
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughResourceBase adds typed DSC test and set results, WhatIf prediction, deletion, export, existence handling, and runtime JSON schema support. New unit and integration tests cover these operations with an in-memory DSC resource fixture. ChangesResourceBase DSC support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DSCAdapter
participant DscBaseTestResource
participant ResourceBase
participant InMemoryStore
DSCAdapter->>DscBaseTestResource: Invoke Get, Test, Set, Delete, or Export
DscBaseTestResource->>ResourceBase: Delegate lifecycle operation
ResourceBase->>InMemoryStore: Read or update resource state
ResourceBase-->>DscBaseTestResource: Return typed result or instances
DscBaseTestResource-->>DSCAdapter: Return DSC operation response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
|
@gaelcolas - after trying out v0.119.1 of Sampler locally, the build succeeded. Looks like there have been some breaking changes from v0.120.0 onwards? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
tests/Unit/Private/New-DscResultTuple.Tests.ps1 (1)
105-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert against the localized string.
The test hardcodes the English error text. Build the expected message from
$script:localizedDataso a change of the string does not silently break the intent of the test.♻️ Proposed change
Context 'When the number of types does not match the number of values' { It 'Should throw the correct error' { InModuleScope -ScriptBlock { - { New-DscResultTuple -Type @([System.String]) -Value @('MyValue', 'MySecondValue') } | - Should -Throw -ExpectedMessage '*does not match the number of values*' + Set-StrictMode -Version 1.0 + + $mockExpectedMessage = $script:localizedData.NewDscResultTuple_CountMismatch -f 1, 2 + + { New-DscResultTuple -Type @([System.String]) -Value @('MyValue', 'MySecondValue') } | + Should -Throw -ExpectedMessage ('*{0}*' -f $mockExpectedMessage) } } }As per path instructions: "Test with localized strings: Use
InModuleScope -ScriptBlock { $script:localizedData.Key }".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Private/New-DscResultTuple.Tests.ps1` around lines 105 - 112, Update the “When the number of types does not match the number of values” test for New-DscResultTuple to obtain the expected message from $script:localizedData inside InModuleScope, then assert the thrown error against that localized value instead of hardcoding English text.Source: Path instructions
tests/Integration/ResourceBase.Integration.Tests.ps1 (2)
199-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one
Describeblock per file.The file now contains two
Describeblocks. Move thedsc.exetests into aContextblock insideDescribe 'ResourceBase'and keep the tag and the skip condition on thatContext.♻️ Proposed structure change
-Describe 'ResourceBase with dsc.exe' -Tag 'RequiresDsc' -Skip:$script:skipDscExe { - Context 'When invoking operations through the DSC PowerShell adapter' { + Context 'When invoking operations through the DSC PowerShell adapter' -Tag 'RequiresDsc' -Skip:$script:skipDscExe {As per path instructions: "One
Describeblock per file matching the tested entity name".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Integration/ResourceBase.Integration.Tests.ps1` around lines 199 - 200, Consolidate the dsc.exe test suite into the existing Describe 'ResourceBase' block instead of declaring a second Describe. Wrap these tests in a Context that retains the RequiresDsc tag and skip:$script:skipDscExe condition, while preserving the existing test behavior.Source: Path instructions
96-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNote the order dependency between the contexts.
The
Export(),Set(), andDelete()contexts share one in-memory store. TheExport()test expects two instances, and theDelete()test removesInstance1. A change of test order, or execution of a singleContext, then fails. Reset the fixture state in aBeforeEachorBeforeAllblock perContextto make each context independent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Integration/ResourceBase.Integration.Tests.ps1` around lines 96 - 166, Reset the shared in-memory fixture before each relevant context so the tests under Export(), Set(), and Delete() start from the expected initial state independently. Add the setup to the appropriate BeforeEach or BeforeAll blocks, using the existing fixture initialization mechanism, while preserving each context’s current assertions and behavior.source/Private/ConvertTo-JsonSchemaTypeDefinition.ps1 (1)
1-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
.INPUTSsection in the comment-based help of the new private functions. All three new functions declare.OUTPUTSbut omit.INPUTS. None of them accept pipeline input, so each help block must declareNone..
source/Private/ConvertTo-JsonSchemaTypeDefinition.ps1#L1-L22: add an.INPUTSsection withNone.before.OUTPUTS.source/Private/ConvertTo-DscResourceJsonSchema.ps1#L1-L22: add an.INPUTSsection withNone.before.OUTPUTS.source/Private/New-DscResultTuple.ps1#L1-L39: add an.INPUTSsection withNone.before.OUTPUTS.As per path instructions: "INPUTS: List each pipeline‑accepted type as inline code with a 1‑line description. ... If there are no inputs, specify
None.."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/Private/ConvertTo-JsonSchemaTypeDefinition.ps1` around lines 1 - 22, Update the comment-based help for ConvertTo-JsonSchemaTypeDefinition in source/Private/ConvertTo-JsonSchemaTypeDefinition.ps1#L1-L22, ConvertTo-DscResourceJsonSchema in source/Private/ConvertTo-DscResourceJsonSchema.ps1#L1-L22, and New-DscResultTuple in source/Private/New-DscResultTuple.ps1#L1-L39 by adding an .INPUTS section containing None. immediately before each .OUTPUTS section.Source: Path instructions
source/Private/New-DscResultTuple.ps1 (1)
57-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
$PSCmdlet.ThrowTerminatingError()instead ofthrow.The guidelines require terminating errors from functions to use
$PSCmdlet.ThrowTerminatingError()withNew-ErrorRecordandNew-Exception. Also check the localized key name against the patternVerb_FunctionName_Action;NewDscResultTuple_CountMismatchmisses the separator after the verb.♻️ Proposed error handling change
if ($Type.Count -ne $Value.Count) { - throw ($script:localizedData.NewDscResultTuple_CountMismatch -f $Type.Count, $Value.Count) + $errorMessage = $script:localizedData.New_DscResultTuple_CountMismatch -f $Type.Count, $Value.Count + + $PSCmdlet.ThrowTerminatingError( + (New-ErrorRecord -Message $errorMessage -ErrorId 'NDRT0001' -ErrorCategory 'InvalidArgument' -TargetObject $Type) + ) }If you rename the key, update
source/en-US/DscResource.Base.strings.psd1and the assertion intests/Unit/Private/New-DscResultTuple.Tests.ps1.As per path instructions: "Use
$PSCmdlet.ThrowTerminatingError()for terminating errors (except for classes), use relevant error category, in try-catch include exception with localized message" and "Format:Verb_FunctionName_Action(underscore separators)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/Private/New-DscResultTuple.ps1` around lines 57 - 60, Replace the direct throw in New-DscResultTuple with $PSCmdlet.ThrowTerminatingError(), constructing the error through New-Exception and New-ErrorRecord with the appropriate error category. Rename the localized key to follow the Verb_FunctionName_Action pattern, then update its definition in DscResource.Base.strings.psd1 and the corresponding assertion in New-DscResultTuple.Tests.ps1.Source: Path instructions
tests/Unit/Private/ConvertTo-DscResourceJsonSchema.Tests.ps1 (1)
120-234: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGroup the
Itblocks inContextblocks.All
Itblocks sit directly underDescribe. AddContextblocks per scenario, for example aContext 'When converting a class-based DSC resource type'block that wraps the schema document tests, and separateContextblocks for the property conversion scenarios.As per path instructions: "Each scenario = separate
Contextblock" and "Contextdescriptions start with 'When'".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Private/ConvertTo-DscResourceJsonSchema.Tests.ps1` around lines 120 - 234, Group the direct child tests in the ConvertTo-DscResourceJsonSchema Describe block into separate Context blocks, with each scenario in its own Context and every Context description starting with “When”. Use a class-based resource Context for the schema document keyword tests and separate “When” Contexts for each property conversion, inheritance, exclusion, and description scenario; keep the existing It assertions unchanged.Source: Path instructions
tests/Unit/Private/ConvertTo-JsonSchemaTypeDefinition.Tests.ps1 (1)
60-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
Set-StrictMode -Version 1.0in the newInModuleScopeunit tests. All three new unit test files call the private function insideInModuleScopewithout strict mode. Add the statement immediately before each invocation.
tests/Unit/Private/ConvertTo-JsonSchemaTypeDefinition.Tests.ps1#L60-L64: addSet-StrictMode -Version 1.0before eachConvertTo-JsonSchemaTypeDefinitioncall in everyInModuleScopeblock.tests/Unit/Private/ConvertTo-DscResourceJsonSchema.Tests.ps1#L121-L125: addSet-StrictMode -Version 1.0before eachConvertTo-DscResourceJsonSchemacall in everyInModuleScopeblock.tests/Unit/Private/New-DscResultTuple.Tests.ps1#L48-L59: addSet-StrictMode -Version 1.0before eachNew-DscResultTuplecall in everyInModuleScopeblock.As per path instructions: "In
InModuleScopetests, addSet-StrictMode -Version 1.0immediately before invoking the tested function".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Private/ConvertTo-JsonSchemaTypeDefinition.Tests.ps1` around lines 60 - 64, All new InModuleScope unit tests must enable strict mode immediately before invoking the tested private function. In tests/Unit/Private/ConvertTo-JsonSchemaTypeDefinition.Tests.ps1 lines 60-64, add Set-StrictMode -Version 1.0 before every ConvertTo-JsonSchemaTypeDefinition call; apply the same change before every ConvertTo-DscResourceJsonSchema call in tests/Unit/Private/ConvertTo-DscResourceJsonSchema.Tests.ps1 lines 121-125 and every New-DscResultTuple call in tests/Unit/Private/New-DscResultTuple.Tests.ps1 lines 48-59.Source: Path instructions
source/Classes/010.ResourceBase.ps1 (1)
356-359: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew
ResourceBasemethods usethrowfor terminating errors. The class guidelines requireNew-*Exceptioncommands for terminating errors in classes. Both new methods raise errors withthrow.
source/Classes/010.ResourceBase.ps1#L356-L359: replacethrowinDeleteInstance()withNew-InvalidOperationException(orNew-NotImplementedException) using the localizedDeleteInstanceNotSupportedmessage.source/Classes/010.ResourceBase.ps1#L375-L378: replacethrowinExportInstances()withNew-NotImplementedExceptionusing the localizedExportInstancesMethodNotImplementedmessage.As per coding guidelines: "Do not use
throwfor terminating errors, useNew-*Exceptioncommands (never for functions)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/Classes/010.ResourceBase.ps1` around lines 356 - 359, Replace the terminating throw in ResourceBase.DeleteInstance() at source/Classes/010.ResourceBase.ps1:356-359 with New-InvalidOperationException or New-NotImplementedException, preserving the localized DeleteInstanceNotSupported message. Also replace the terminating throw in ResourceBase.ExportInstances() at source/Classes/010.ResourceBase.ps1:375-378 with New-NotImplementedException using the localized ExportInstancesMethodNotImplemented message.Source: Path instructions
tests/Unit/Classes/ResourceBase.Tests.ps1 (2)
1786-1811: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the
Itblocks in aContextblock.The
Itblocks sit directly in theDescribeblock. The tests guidelines require a separateContextblock per scenario, and the description must start with 'When'. TheGetInstanceJsonSchema()Describeat Lines 2116-2149 has the same structure.As per coding guidelines: "Each scenario = separate
Contextblock" and "Contextdescriptions start with 'When'".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Classes/ResourceBase.Tests.ps1` around lines 1786 - 1811, Wrap the three `It` blocks testing `GetPredictedState` in a dedicated `Context` whose description starts with “When”, keeping each assertion-focused test within that context. Apply the same structure to the `GetInstanceJsonSchema()` `Describe` block, using a separate “When” context for its scenario tests.Source: Path instructions
1969-1973: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert against the localized strings.
Both assertions use hardcoded message fragments. Read the message through
$script:localizedDataso a message change does not silently break the intent of the test.As per coding guidelines: "Test with localized strings: Use
InModuleScope -ScriptBlock { $script:localizedData.Key }".Also applies to: 1992-1994
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Classes/ResourceBase.Tests.ps1` around lines 1969 - 1973, Update the error assertions in the tests around DeleteInstance and the additional assertion near the referenced range to use the expected message fragments from $script:localizedData inside InModuleScope, rather than hardcoded localized text. Preserve the existing wildcard matching and exception behavior while referencing the appropriate localization keys.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/en-US/ResourceBase.strings.psd1`:
- Line 18: Update the DeleteInstanceNotSupported resource string to state that
delete is unsupported when the resource lacks both the _exist and Ensure
properties, while preserving the existing DeleteInstance() override guidance and
error code.
In `@source/Private/New-DscResultTuple.ps1`:
- Around line 62-64: Add an arity validation guard before constructing
$closedTupleType in New-DscResultTuple, rejecting $Type.Count values above 8
with the existing localized count-mismatch/validation error. Preserve the
current generic tuple creation flow for supported arities from 1 through 8.
In `@tests/Integration/ResourceBase.Integration.Tests.ps1`:
- Around line 29-35: Update the PSModulePath setup near $script:fixturePath to
also prepend the built module’s output\RequiredModules directory before
importing DscResourceBaseTestResource. Preserve the existing Fixtures path and
ordering, and ensure both paths are included in $env:PSModulePath for child
processes such as dsc.exe.
In `@tests/Unit/Classes/ResourceBase.Tests.ps1`:
- Around line 2116-2122: Update the test for
$mockResourceBaseType::InstanceJsonSchema() to invoke ConvertFrom-Json directly
with -ErrorAction 'Stop', removing the surrounding Should -Not -Throw assertion
while retaining validation of the returned JSON.
---
Nitpick comments:
In `@source/Classes/010.ResourceBase.ps1`:
- Around line 356-359: Replace the terminating throw in
ResourceBase.DeleteInstance() at source/Classes/010.ResourceBase.ps1:356-359
with New-InvalidOperationException or New-NotImplementedException, preserving
the localized DeleteInstanceNotSupported message. Also replace the terminating
throw in ResourceBase.ExportInstances() at
source/Classes/010.ResourceBase.ps1:375-378 with New-NotImplementedException
using the localized ExportInstancesMethodNotImplemented message.
In `@source/Private/ConvertTo-JsonSchemaTypeDefinition.ps1`:
- Around line 1-22: Update the comment-based help for
ConvertTo-JsonSchemaTypeDefinition in
source/Private/ConvertTo-JsonSchemaTypeDefinition.ps1#L1-L22,
ConvertTo-DscResourceJsonSchema in
source/Private/ConvertTo-DscResourceJsonSchema.ps1#L1-L22, and
New-DscResultTuple in source/Private/New-DscResultTuple.ps1#L1-L39 by adding an
.INPUTS section containing None. immediately before each .OUTPUTS section.
In `@source/Private/New-DscResultTuple.ps1`:
- Around line 57-60: Replace the direct throw in New-DscResultTuple with
$PSCmdlet.ThrowTerminatingError(), constructing the error through New-Exception
and New-ErrorRecord with the appropriate error category. Rename the localized
key to follow the Verb_FunctionName_Action pattern, then update its definition
in DscResource.Base.strings.psd1 and the corresponding assertion in
New-DscResultTuple.Tests.ps1.
In `@tests/Integration/ResourceBase.Integration.Tests.ps1`:
- Around line 199-200: Consolidate the dsc.exe test suite into the existing
Describe 'ResourceBase' block instead of declaring a second Describe. Wrap these
tests in a Context that retains the RequiresDsc tag and skip:$script:skipDscExe
condition, while preserving the existing test behavior.
- Around line 96-166: Reset the shared in-memory fixture before each relevant
context so the tests under Export(), Set(), and Delete() start from the expected
initial state independently. Add the setup to the appropriate BeforeEach or
BeforeAll blocks, using the existing fixture initialization mechanism, while
preserving each context’s current assertions and behavior.
In `@tests/Unit/Classes/ResourceBase.Tests.ps1`:
- Around line 1786-1811: Wrap the three `It` blocks testing `GetPredictedState`
in a dedicated `Context` whose description starts with “When”, keeping each
assertion-focused test within that context. Apply the same structure to the
`GetInstanceJsonSchema()` `Describe` block, using a separate “When” context for
its scenario tests.
- Around line 1969-1973: Update the error assertions in the tests around
DeleteInstance and the additional assertion near the referenced range to use the
expected message fragments from $script:localizedData inside InModuleScope,
rather than hardcoded localized text. Preserve the existing wildcard matching
and exception behavior while referencing the appropriate localization keys.
In `@tests/Unit/Private/ConvertTo-DscResourceJsonSchema.Tests.ps1`:
- Around line 120-234: Group the direct child tests in the
ConvertTo-DscResourceJsonSchema Describe block into separate Context blocks,
with each scenario in its own Context and every Context description starting
with “When”. Use a class-based resource Context for the schema document keyword
tests and separate “When” Contexts for each property conversion, inheritance,
exclusion, and description scenario; keep the existing It assertions unchanged.
In `@tests/Unit/Private/ConvertTo-JsonSchemaTypeDefinition.Tests.ps1`:
- Around line 60-64: All new InModuleScope unit tests must enable strict mode
immediately before invoking the tested private function. In
tests/Unit/Private/ConvertTo-JsonSchemaTypeDefinition.Tests.ps1 lines 60-64, add
Set-StrictMode -Version 1.0 before every ConvertTo-JsonSchemaTypeDefinition
call; apply the same change before every ConvertTo-DscResourceJsonSchema call in
tests/Unit/Private/ConvertTo-DscResourceJsonSchema.Tests.ps1 lines 121-125 and
every New-DscResultTuple call in tests/Unit/Private/New-DscResultTuple.Tests.ps1
lines 48-59.
In `@tests/Unit/Private/New-DscResultTuple.Tests.ps1`:
- Around line 105-112: Update the “When the number of types does not match the
number of values” test for New-DscResultTuple to obtain the expected message
from $script:localizedData inside InModuleScope, then assert the thrown error
against that localized value instead of hardcoding English text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f2f5c96-0497-4c8c-9d56-3931cf453935
📒 Files selected for processing (15)
CHANGELOG.mdsource/Classes/010.ResourceBase.ps1source/Private/ConvertTo-DscResourceJsonSchema.ps1source/Private/ConvertTo-JsonSchemaTypeDefinition.ps1source/Private/New-DscResultTuple.ps1source/en-US/DscResource.Base.strings.psd1source/en-US/ResourceBase.strings.psd1tests/Integration/Fixtures/DscResourceBaseTestResource/DscResourceBaseTestResource.psd1tests/Integration/Fixtures/DscResourceBaseTestResource/DscResourceBaseTestResource.psm1tests/Integration/Fixtures/DscResourceBaseTestResource/en-US/DscBaseTestResource.strings.psd1tests/Integration/ResourceBase.Integration.Tests.ps1tests/Unit/Classes/ResourceBase.Tests.ps1tests/Unit/Private/ConvertTo-DscResourceJsonSchema.Tests.ps1tests/Unit/Private/ConvertTo-JsonSchemaTypeDefinition.Tests.ps1tests/Unit/Private/New-DscResultTuple.Tests.ps1
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
this doesn't look like it would break existing functionality. Just a small comment.
|
Not sure why the build breaks, something change in new Sampler release maybe? |
|
@johlju - is the PR that Raimund opened up on Sampler going to resolve it: gaelcolas/Sampler#594? If I change the |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/Private/New-DscResultTuple.ps1 (1)
26-27: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComplete the output and input contract in comment-based help.
Add
.INPUTSwithNone.because this function does not accept pipeline input. Change[OutputType([System.Object])]and.OUTPUTSto[System.Tuple]. The function always returns a tuple instance.Proposed fix
- [OutputType([System.Object])] + [OutputType([System.Tuple])] ... + .INPUTS + None. + .OUTPUTS - [System.Object] + [System.Tuple]As per path instructions, comment-based help must list
INPUTS, andOUTPUTSmust match both[OutputType()]and actual returns.Also applies to: 86-86
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/Private/New-DscResultTuple.ps1` around lines 26 - 27, Update the comment-based help for New-DscResultTuple by adding an .INPUTS section containing None., and change both its [OutputType()] declaration and .OUTPUTS entry from [System.Object] to [System.Tuple] to match the function’s return value.Source: Path instructions
🧹 Nitpick comments (1)
source/Private/New-DscResultTuple.ps1 (1)
74-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a descriptive loop variable name.
Rename
$ito$index. This name meets the required minimum length and avoids an abbreviation.As per path instructions, use descriptive variable names with at least three characters and no abbreviations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/Private/New-DscResultTuple.ps1` at line 74, In the loop within New-DscResultTuple, rename the abbreviated variable $i to $index and update every reference to it in the loop, preserving the existing iteration behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@source/Private/New-DscResultTuple.ps1`:
- Around line 26-27: Update the comment-based help for New-DscResultTuple by
adding an .INPUTS section containing None., and change both its [OutputType()]
declaration and .OUTPUTS entry from [System.Object] to [System.Tuple] to match
the function’s return value.
---
Nitpick comments:
In `@source/Private/New-DscResultTuple.ps1`:
- Line 74: In the loop within New-DscResultTuple, rename the abbreviated
variable $i to $index and update every reference to it in the loop, preserving
the existing iteration behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c6b6d567-0550-49e0-b5a1-6b6da7ab406d
📒 Files selected for processing (2)
build.yamlsource/Private/New-DscResultTuple.ps1
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
===================================
Coverage 99% 99%
===================================
Files 7 10 +3
Lines 133 256 +123
===================================
+ Hits 132 255 +123
Misses 1 1
🚀 New features to boost your workflow:
|
|
@Gijsreyn Let us see when that PR in Sampler is merged, until then can you see if you can resolve the failing tests. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
source/Private/New-DscResultTuple.ps1 (2)
1-39: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComplete the comment-based help.
Add an
.INPUTSsection withNone.because no parameter accepts pipeline input. Add a one-line description below[System.Object]in.OUTPUTS.As per path instructions: comment-based help must include SYNOPSIS, DESCRIPTION, PARAMETER, EXAMPLE, INPUTS, and OUTPUTS sections, with descriptions for each input and output.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/Private/New-DscResultTuple.ps1` around lines 1 - 39, Complete the comment-based help for New-DscResultTuple by adding an .INPUTS section stating None., and add a one-line description under .OUTPUTS explaining the returned object. Preserve the existing SYNOPSIS, DESCRIPTION, PARAMETER, and EXAMPLE sections.Source: Path instructions
63-66: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a structured terminating error for count mismatches.
At line 65,
throwcreates a generic argument exception without a relevant category or stable error ID. Replace it with a localizedErrorRecordthat usesDRB0004,InvalidArgument, the relevant values as target objects, and pass the record to$PSCmdlet.ThrowTerminatingError(). Use the available DscResource.Common helpers if available rather than introducing directErrorRecordconstruction unless those helpers are not imported.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/Private/New-DscResultTuple.ps1` around lines 63 - 66, Update the count-mismatch branch in New-DscResultTuple to create a localized ErrorRecord with error ID DRB0004, InvalidArgument category, and the relevant Type.Count and Value.Count as target objects, then terminate via $PSCmdlet.ThrowTerminatingError(). Reuse an available DscResource.Common error helper; only construct ErrorRecord directly if the helper is not imported.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@source/Private/New-DscResultTuple.ps1`:
- Around line 1-39: Complete the comment-based help for New-DscResultTuple by
adding an .INPUTS section stating None., and add a one-line description under
.OUTPUTS explaining the returned object. Preserve the existing SYNOPSIS,
DESCRIPTION, PARAMETER, and EXAMPLE sections.
- Around line 63-66: Update the count-mismatch branch in New-DscResultTuple to
create a localized ErrorRecord with error ID DRB0004, InvalidArgument category,
and the relevant Type.Count and Value.Count as target objects, then terminate
via $PSCmdlet.ThrowTerminatingError(). Reuse an available DscResource.Common
error helper; only construct ErrorRecord directly if the helper is not imported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c9021975-09a1-4c7c-9b58-9c3a60ac14a2
📒 Files selected for processing (1)
source/Private/New-DscResultTuple.ps1
|
@johlju - the tests are green, sir! Perhaps a look from @gaelcolas is also good to have 😄 |
|
I need to catch up with the DSCv3 work (RFC/Adapter) before I can review this PR correctly. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
Pull Request (PR) description
Adds DSC v3 (Microsoft DSC) support to
ResourceBase. New hidden helpermethods (
GetTestResult(),GetSetResult(),DeleteInstance(),ExportInstances(),GetInstanceJsonSchema()) let a derived classparticipate in Microsoft DSC semantics by declaring one-liner static methods.
These are added as per the resource contract RFC while remaining compatible with PSDSC v1/v2.
This Pull Request (PR) fixes the following issues
n/a
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