PowerPlan: Windows APIs instead of WMI (alternative for PR #202) #203

J0F3 · 2019-01-24T15:47:41Z

Pull Request (PR) description

This is basically an other variant of the changes in PR #202. This one uses however Windows Native APIs instead of powercfg.exe as a replacement of the WMI classes.
Created this one to have both variants available here with the idea to choose eventually the one to go forward with...

This Pull Request (PR) fixes the following issues

Task list

Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
Entry should say what was changed, and how that affects users (if applicable).
[ N/A] Resource documentation added/updated in README.md in the resource folder.
Resource parameter descriptions added/updated in schema.mof
and comment-based help.
Comment-based help added/updated.
Localization strings added/updated in all localization files as appropriate.
[N/A] Examples appropriately added/updated.
Unit tests added/updated. See DSC Resource Testing Guidelines.
Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is

…unctions

J0F3 · 2019-01-24T15:54:46Z

The resource basically works with calling the Windows APIs but it is probably still some work/refactoring to do here. For example I am basically a little stuck while creating unit tests for the new Get-PowerPlan, Get-PowerPlanFriendlyName, Get-ActivePowerPlan and Set-ActivePowerPlan because of the nature that they call all the P/Invoke classes and static methods...

PlagueHO · 2019-01-25T09:33:14Z

I love this @J0F3 solution! +++ great. I saw your e-mails in the MVP PGI groups about this and watched with great interest and was glad to see that it was identified as a bug. But I like your solution even better. I'll review this weekend (getting a bit late tonight).

PlagueHO · 2019-02-16T22:30:15Z

Hi @J0F3 - I'll try and get to this one as soon as I can. But be aware, I'm going to start work on refactoring this module to move away from the test harness and enable example publishing to PS Gallery. So this will change the layout of the folders in the module significantly and might have cause conflicts with any open PRs.

PlagueHO · 2019-03-01T09:16:02Z

Hi @J0F3 - any chance you could resolve the conflicts on this one and I'll try and get the review done this weekend?

codecov-io · 2019-03-04T06:15:34Z

Codecov Report

Merging #203 into dev will decrease coverage by <1%.
The diff coverage is 100%.

@@        Coverage Diff         @@
##           dev   #203   +/-   ##
==================================
- Coverage   90%    90%   -1%     
==================================
  Files        8      8           
  Lines      844    836    -8     
==================================
- Hits       762    754    -8     
  Misses      82     82

J0F3 · 2019-03-04T06:23:17Z

@PlagueHO just resolved the conflicts. Was an easy one because there was only one conflict in the CHANGELOG.md... 😊
I thought also about some refactoring and to capsuling the calls of the native API in to separate functions. So it would be probably a little better testable. But for that I had not the time yet...

PlagueHO · 2019-03-05T07:17:38Z

Cool. Thanks @J0F3 - I'll get onto this tonight/tomorrow. Just a super busy week with day-job this week 😢

J0F3 · 2019-03-06T17:28:45Z

@PlagueHO now worries and hurry. I know how it is... 🙂

PlagueHO

Reviewed 7 of 9 files at r1, 1 of 1 files at r3.
Reviewable status: 8 of 9 files reviewed, 40 unresolved discussions (waiting on @J0F3 and @PlagueHO)

Modules/ComputerManagementDsc/DSCResources/MSFT_PowerPlan/MSFT_PowerPlan.psm1, line 51 at r3 (raw file):

    $desiredPowerPlan = Get-PowerPlan -PowerPlan $Name

    if($desiredPowerPlan)

Add space after if

Modules/ComputerManagementDsc/DSCResources/MSFT_PowerPlan/MSFT_PowerPlan.psm1, line 54 at r3 (raw file):

    {
        $activePowerPlan = Get-ActivePowerPlan
        if($activePowerPlan -eq $desiredPowerPlan.Guid)

Add blank line before this one and add space after if

Modules/ComputerManagementDsc/DSCResources/MSFT_PowerPlan/MSFT_PowerPlan.psm1, line 58 at r3 (raw file):

            Write-Verbose -Message ($script:localizedData.PowerPlanIsActive -f $desiredPowerPlan.FriendlyName)

            return @{

Could you move this block to after if/else block and just create an $isActive variable?

That would reduce code duplication.

E.g.

            return @{
                IsSingleInstance = $IsSingleInstance
                Name             = $Name
                IsActive         = $isActive
}

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 500 at r3 (raw file):

        the friendly name and GUID of the power plan(s).

    .PARAMETER PowerPlanFriendlyName

Should be PowerPlan

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 506 at r3 (raw file):

    .NOTES
        This function uses Platform Invoke (P/Invoke) mechanism to call native Windows APIs
        because the Win32_PowerPlan WMI class has on some platforms issues or is unavailable at all.

Nitpick: has on some platforms issues -> has issues on some platforms

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 507 at r3 (raw file):

        This function uses Platform Invoke (P/Invoke) mechanism to call native Windows APIs
        because the Win32_PowerPlan WMI class has on some platforms issues or is unavailable at all.
        (e.g Server 2012 R2 core or Nano Server).

No need for brackets 😁

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 510 at r3 (raw file):

        This function is used by the PowerPlan resource.
#>
function Get-PowerPlan {

{ needs to be on next line.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 511 at r3 (raw file):

#>
function Get-PowerPlan {
    [CmdletBinding()]

Can you add an [OutputType([[ ])] attribute which whatever the return type is?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 545 at r3 (raw file):

    $index = 0
    $returnCode = 0
    $guids = @()

Suggest using [System.Collections.ArrayList] as it actually has better performance than @().

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 550 at r3 (raw file):

    $bufferSize = 16

    # The PowerEnumerate function returns only one guid at a time.

Can you use <# #> for block comments?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 552 at r3 (raw file):

    # The PowerEnumerate function returns only one guid at a time.
    # So we have to loop here until error code 259 (no more data) is returned to get all power plan guids.
    while ($returnCode -eq 0)

Is it also possible to break out the content of the while block into a separate function?

E.g. Something like:
Get-AllAvailablePowerPlanGuid

This will simplify unit testing a little bit as we can then mock this out

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 554 at r3 (raw file):

    while ($returnCode -eq 0)
    {
        try {

Can you move { to the next line?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 556 at r3 (raw file):

        try {
            # Allocate buffer
            $readBuffer = [System.Runtime.InteropServices.Marshal]::AllocHGlobal([int]$bufferSize)

Rather than [int] can we use [System.Int32]?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 562 at r3 (raw file):

            # Create a managed Guid object form the unmanaged memory block
            $planGuid = [System.Runtime.InteropServices.Marshal]::PtrToStructure($readBuffer, [System.Type][guid])

Rather than [guid] can we use [System.Guid] (trying to avoid type accelerators).

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 565 at r3 (raw file):

            # ReturnCode 259 from the native function means no more data
            if ($ReturnCode -eq 259) {

Can you move { to the next line? And also lower case r for $returnCode

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 578 at r3 (raw file):

            }

            $guids += $planGuid

If using [System.Collections.ArrayList] then would use $null = $guids.Add($planGuid)

It also might be worth adding some Verbose messages around here so we can help users track down issues (hopefully there are none, but as this is quite a reworking of how we do PowerPlans we might want some additional info).

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 588 at r3 (raw file):

    # Now get the friendly name for each power plan so we can filter on name if needed.
    $allAvailablePowerPlans = @()

Why not do this in the while block above? Instead of creating an array of guids and then creating a new array and looping through it. This would reduce code a bit.

It would mean you'd probably need the while block in a function like:
Get-AllAvailablePowerPlan

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 589 at r3 (raw file):

    # Now get the friendly name for each power plan so we can filter on name if needed.
    $allAvailablePowerPlans = @()
    foreach($planGuid in $guids)

Can you add space after foreach?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 600 at r3 (raw file):

    # If a specific power plan is specified filter for it.
    if($PSBoundParameters.ContainsKey('PowerPlan')){

Can you add space after if and put { on next line?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 623 at r3 (raw file):

    .NOTES
        This function uses Platform Invoke (P/Invoke) mechanism to call native Windows APIs
        because the Win32_PowerPlan WMI class has on some platforms issues or is unavailable at all.

See previous suggested wording.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 629 at r3 (raw file):

function Get-PowerPlanFriendlyName
{
    [CmdletBinding()]

Can you add an [OutputType([[ ])] attribute which whatever the return type is?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 666 at r3 (raw file):

    try
    {
        # Frist get needed buffer size by calling PowerReadFriendlyName

Can you use comment block format here? <# #>

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 668 at r3 (raw file):

        # Frist get needed buffer size by calling PowerReadFriendlyName
        # with NULL value for 'Buffer' parameter to get the required buffer size.
        $returnCode = $powerprof::PowerReadFriendlyName([System.IntPtr]::Zero, $PowerPlanGuid, [System.IntPtr]::Zero, [System.IntPtr]::Zero, [System.IntPtr]::Zero, [ref]$bufferSize)

Any chance of reducing the length of this line? E.g.

$returnCode = $powerprof::PowerReadFriendlyName(`
  [System.IntPtr]::Zero, `
  $PowerPlanGuid, `
  [System.IntPtr]::Zero, `
  [System.IntPtr]::Zero, `
  [System.IntPtr]::Zero, `
  [ref]$bufferSize)

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 675 at r3 (raw file):

            {
                # Now lets allocate the needed buffer size
                $ptrName = [System.Runtime.InteropServices.Marshal]::AllocHGlobal([int]$bufferSize)

Can you use [System.Int32] instead?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 677 at r3 (raw file):

                $ptrName = [System.Runtime.InteropServices.Marshal]::AllocHGlobal([int]$bufferSize)

                # Get the actual friendly name of the powerlan by calling PowerReadFriendlyName again.

Can you use comment block format here? <# #>

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 679 at r3 (raw file):

                # Get the actual friendly name of the powerlan by calling PowerReadFriendlyName again.
                # This time with the correct buffer size for the 'Buffer' parameter.
                $returnCode = $powerprof::PowerReadFriendlyName([System.IntPtr]::Zero, $PowerPlanGuid, [System.IntPtr]::Zero, [System.IntPtr]::Zero, $ptrName, [ref]$bufferSize)

Any chance of reducing the length of this line?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 682 at r3 (raw file):

                if ($returnCode -eq 0)
                {
                    #Create a managed String object form the unmanned memory block.

Can you add space after #

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 688 at r3 (raw file):

                else
                {
                    throw [ComponentModel.Win32Exception]::new([int]$returnCode)

Can you use [System.Int32] instead?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 699 at r3 (raw file):

        else
        {
            throw [ComponentModel.Win32Exception]::new([int]$returnCode)

Can you use [System.Int32] instead?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 715 at r3 (raw file):

    .NOTES
        This function uses Platform Invoke (P/Invoke) mechanism to call native Windows APIs
        because the Win32_PowerPlan WMI class has on some platforms issues or is unavailable at all.

See previous suggested wording.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 716 at r3 (raw file):

        This function uses Platform Invoke (P/Invoke) mechanism to call native Windows APIs
        because the Win32_PowerPlan WMI class has on some platforms issues or is unavailable at all.
        (e.g Server 2012 R2 core or Nano Server).

No need for brackets around this line 😁

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 721 at r3 (raw file):

function Get-ActivePowerPlan
{
    [CmdletBinding()]

Can you add an [OutputType([[System.Guid])] attribute ?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 753 at r3 (raw file):

        {
            # Create a Win32Exception object out of the return code
            $win32Exception = ([ComponentModel.Win32Exception]::new([int]$returnCode))

Can we use [System.Int32]?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 759 at r3 (raw file):

        # Create a managed Guid object form the unmanaged memory block and return it
        return [System.Runtime.InteropServices.Marshal]::PtrToStructure($activeSchemeGuid, [System.Type][guid])

Can we use [System.Guid]?

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 777 at r3 (raw file):

    .NOTES
        This function uses Platform Invoke (P/Invoke) mechanism to call native Windows APIs
        because the Win32_PowerPlan WMI class has on some platforms issues or is unavailable at all.

See previous suggested wording.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 778 at r3 (raw file):

        This function uses Platform Invoke (P/Invoke) mechanism to call native Windows APIs
        because the Win32_PowerPlan WMI class has on some platforms issues or is unavailable at all.
        (e.g Server 2012 R2 core or Nano Server).

No need for brackets around this line 😁

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 781 at r3 (raw file):

        This function is used by the PowerPlan resource.
#>
function Set-ActivePowerPlan {

{ needs to be on next line.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 803 at r3 (raw file):

    # Create Win32PowerSetActiveScheme object with the static method PowerSetActiveScheme.
    $powrprof = Add-Type -MemberDefinition $powerSetActiveSchemeDefinition -Name 'Win32PowerSetActiveScheme' -Namespace 'Win32Functions' -PassThru

Can you reduce the line length? E.g.

$powrprof = Add-Type `
  -MemberDefinition $powerSetActiveSchemeDefinition `
  -Name 'Win32PowerSetActiveScheme' `
  -Namespace 'Win32Functions' `
  -PassThru

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 805 at r3 (raw file):

    $powrprof = Add-Type -MemberDefinition $powerSetActiveSchemeDefinition -Name 'Win32PowerSetActiveScheme' -Namespace 'Win32Functions' -PassThru

    try {

{ needs to be on next line.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 816 at r3 (raw file):

        }
    }
    catch {

{ needs to be on next line.

J0F3

Reviewable status: 5 of 10 files reviewed, 40 unresolved discussions (waiting on @PlagueHO)

Modules/ComputerManagementDsc/DSCResources/MSFT_PowerPlan/MSFT_PowerPlan.psm1, line 51 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Add space after if

Done.

Modules/ComputerManagementDsc/DSCResources/MSFT_PowerPlan/MSFT_PowerPlan.psm1, line 54 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Add blank line before this one and add space after if

Done.

Modules/ComputerManagementDsc/DSCResources/MSFT_PowerPlan/MSFT_PowerPlan.psm1, line 58 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you move this block to after if/else block and just create an $isActive variable?

That would reduce code duplication.

E.g.

            return @{
                IsSingleInstance = $IsSingleInstance
                Name             = $Name
                IsActive         = $isActive
}

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 500 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Should be PowerPlan

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 506 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: has on some platforms issues -> has issues on some platforms

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 507 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

No need for brackets 😁

😉
Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 510 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

{ needs to be on next line.

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 511 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add an [OutputType([[ ])] attribute which whatever the return type is?

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 545 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Suggest using [System.Collections.ArrayList] as it actually has better performance than @().

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 550 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use <# #> for block comments?

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 552 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Is it also possible to break out the content of the while block into a separate function?

E.g. Something like:
Get-AllAvailablePowerPlanGuid

This will simplify unit testing a little bit as we can then mock this out

Wrote some test for Get-PowerPlan now which mockes out the new Get-PowerPlanUsingPInvoke function

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 554 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you move { to the next line?

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 556 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Rather than [int] can we use [System.Int32]?

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 562 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Rather than [guid] can we use [System.Guid] (trying to avoid type accelerators).

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 565 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you move { to the next line? And also lower case r for $returnCode

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 578 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

If using [System.Collections.ArrayList] then would use $null = $guids.Add($planGuid)

It also might be worth adding some Verbose messages around here so we can help users track down issues (hopefully there are none, but as this is quite a reworking of how we do PowerPlans we might want some additional info).

I added also some verbose messages in the new Get-PowerPlanUsingPInvoke function

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 588 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Why not do this in the while block above? Instead of creating an array of guids and then creating a new array and looping through it. This would reduce code a bit.

It would mean you'd probably need the while block in a function like:
Get-AllAvailablePowerPlan

I added a new function (Get-PowerPlanUsingPInvoke) with the PInvoke code which gets also the friendly name in the while block.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 589 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add space after foreach?

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 600 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add space after if and put { on next line?

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 623 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

See previous suggested wording.

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 629 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add an [OutputType([[ ])] attribute which whatever the return type is?

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 666 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use comment block format here? <# #>

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 668 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Any chance of reducing the length of this line? E.g.

$returnCode = $powerprof::PowerReadFriendlyName(`
  [System.IntPtr]::Zero, `
  $PowerPlanGuid, `
  [System.IntPtr]::Zero, `
  [System.IntPtr]::Zero, `
  [System.IntPtr]::Zero, `
  [ref]$bufferSize)
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 675 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VIYvIAgM72c8dy3ct:-L_ZHd2tD-o6CkMU7D8S:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L675)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Can you use [System.Int32] instead?
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 677 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VIMgj5DFKAo90dxgj:-L_ZHX2B5aKW8K3HFQbs:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L677)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Can you use comment block format here? <# #>
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 679 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VIRVkAmQaqBQCfAZV:-L_ZHNde2h99uR75qRoq:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L679)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Any chance of reducing the length of this line?
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 682 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VIezsEZcCVhN6lm49:-L_ZGohVDOgOdGO-xkS6:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L682)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Can you add space after # 
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 688 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VIbyX4HAYmsvjeG7F:-L_ZGjEz4FKkf_RxD1dH:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L688)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Can you use [System.Int32] instead?
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 699 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VIiw46_2QlIUsfDMo:-L_ZGeK41EkA-B4pAox6:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L699)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Can you use [System.Int32] instead?
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 715 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VJuxaEvLDKIqt5Zq7:-L_ZGTPs0sbA752n81e0:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L715)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

See previous suggested wording.
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 716 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VJpvV3rD7EPaVWyzH:-L_ZGSrR330F6WxO1UH8:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L716)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

No need for brackets around this line :grin:
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 721 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VKL2N7ygQQcJ-D9ci:-L_ZGNA_2CG_jgQDIcZ5:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L721)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Can you add an [OutputType([[System.Guid])] attribute ?
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 753 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VK8Uv9e7xGK-rLnTR:-L_ZFtT5EqG9dZuwlgSw:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L753)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Can we use [System.Int32]?
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 759 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VKBOc0pIFkHrjJgw9:-L_ZFpq6DT4Veotx4G_W:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L759)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Can we use [System.Guid]?
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 777 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VKFjD5dyhxZ5XuB7J:-L_ZFkjSBYBVhQ54G-pM:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L777)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

See previous suggested wording.
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 778 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VKIXI4ycCXdY11jYV:-L_ZFkHX0L-B3RDY69r8:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L778)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

No need for brackets around this line :grin:
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 781 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VKuFT1A1HsIhu6aTC:-L_ZFGpfDPWUramsm3Yv:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L781)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

{ needs to be on next line.
</blockquote></details>

Done.
___
*[Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 803 at r3](https://reviewable.io/reviews/powershell/computermanagementdsc/203#-L_VKzVQ8dR7qoRqSCl9:-L_ZF9vJ3iwSBQRLMagl:b-896fix) ([raw file](https://github.com/powershell/computermanagementdsc/blob/771979c7133404f9fc40bfba083d9a83c53572b4/Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1#L803)):*
<details><summary><i>Previously, PlagueHO (Daniel Scott-Raynsford) wrote…</i></summary><blockquote>

Can you reduce the line length? E.g.
```powershell
$powrprof = Add-Type `
  -MemberDefinition $powerSetActiveSchemeDefinition `
  -Name 'Win32PowerSetActiveScheme' `
  -Namespace 'Win32Functions' `
  -PassThru

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 805 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

{ needs to be on next line.

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 816 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

{ needs to be on next line.

Done.

PlagueHO

Sorry about the delay. Day job has been getting in the way. I've not quite finished the review (one file to go), but have run out of time today (got to catch a flight).

Really awesome stuff BTW - can't wait to get this in!

Reviewed 1 of 9 files at r1, 3 of 4 files at r4.
Reviewable status: 9 of 10 files reviewed, 26 unresolved discussions (waiting on @J0F3)

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 766 at r4 (raw file):

    <#
    The PowerEnumerate function returns only one guid at a time.

Totally minor nitpick:

Can you indent the lines in between the <# #> - and through out:
e.g.

<#
    Line 1
    Line 2
#>

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 789 at r4 (raw file):

            {
                # Create a Win32Exception object out of the return code
                $win32Exception = ([ComponentModel.Win32Exception]::new([int]$returnCode))

Can you use [System.Int32] here (trying to be consistent on not using Type Accelerators)

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 823 at r4 (raw file):

    return $allAvailablePowerPlans

Can you remove blank line?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 57 at r4 (raw file):

                        return @{
                                FriendlyName = 'High performance'
                                Guid = [Guid]'8c5e7fda-e8bf-4a96-9a85-a6e23a8c635c'

Could you change to [System.Guid]?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 64 at r4 (raw file):

                Mock `
                -CommandName Get-ActivePowerPlan `

Could you correct the indenting of this line (and the followingones)

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 66 at r4 (raw file):

                -CommandName Get-ActivePowerPlan `
                -MockWith {
                    return [Guid]'8c5e7fda-e8bf-4a96-9a85-a6e23a8c635c'

Could you change to [System.Guid]?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 73 at r4 (raw file):

            It 'Should return the same values as passed as parameters (power plan specified as <Type>)' -TestCases $testCases {

Could you remove this blank line?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 76 at r4 (raw file):

                param
                (
                    [String]

Could you change to [System.String]?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 101 at r4 (raw file):

                Mock `
                -CommandName Get-ActivePowerPlan `

It looks like the indenting is incorrect here.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 113 at r4 (raw file):

                param
                (
                    [String]

Can you convert to [System.String]

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 148 at r4 (raw file):

        Assert-VerifiableMock
    }
    }

Could you add a blank line here? Also it looks like there is a problem with the indenting of the whole block.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 163 at r4 (raw file):

            Mock `
            -CommandName Set-ActivePowerPlan `

Looks like there is an indenting issue.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 169 at r4 (raw file):

        Context 'When the system is not in the desired present state' {

Can you remove blank line?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 171 at r4 (raw file):

            It 'Should call Get-PowerPlan once (power plan specified as <Type>)' -TestCases $testCases {

Can you remove blank line?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 174 at r4 (raw file):

                param
                (
                    [String]

Change to [System.String]

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 187 at r4 (raw file):

                param
                (
                    [String]

Change to [System.String]

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 193 at r4 (raw file):

                Set-TargetResource -Name $Name -IsSingleInstance 'Yes' -Verbose

                Assert-MockCalled -CommandName Set-ActivePowerPlan -Exactly -Times 1 -Scope It -ModuleName $script:DSCResourceName

It is probably a good idea to check that Set-ActivePowerPlan was called with expected parameters here.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 206 at r4 (raw file):

            It 'Should throw the expected error (power plan specified as <Type>)' -TestCases $testCases {

Can you remove blank line here?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 209 at r4 (raw file):

                param
                (
                    [String]

Can you change to [System.String]?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 230 at r4 (raw file):

                        return @{
                                FriendlyName = 'High performance'
                                Guid = [Guid]'8c5e7fda-e8bf-4a96-9a85-a6e23a8c635c'

Can you change to [System.Guid]- and throughout?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 237 at r4 (raw file):

                Mock `
                -CommandName Get-ActivePowerPlan `

It looks like indenting is wrong here.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 247 at r4 (raw file):

            It 'Should return the the state as present ($true) (power plan specified as <Type>)' -TestCases $testCases {

Can you remove blank line?

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 250 at r4 (raw file):

                param
                (
                    [String]

Change to [System.String]

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 271 at r4 (raw file):

                    -Verifiable

                    Mock `

Indenting issue here.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 281 at r4 (raw file):

            It 'Should return the the state as absent ($false) (power plan specified as <Type>)' -TestCases $testCases {

Remove blank line.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 284 at r4 (raw file):

                param
                (
                    [String]

Change to [System.String]

J0F3 · 2019-03-16T07:37:10Z

No worries. See you probably in Redmond then...? 😉

PlagueHO · 2019-03-16T08:21:21Z

Yep 😁 on the way there now. Free in flight wifi due to flight delay! See you in Redmond then :)

craigeaw · 2019-03-22T18:55:34Z

Any updates on this? Would be great to have this available so we don't have to use a Script resource. Looking forward to it!

PlagueHO · 2019-03-22T20:08:40Z

Thanks for querying this @craigeaw . @J0F3 and I have been away but hopefully this one will get completed soon.

J0F3

Reviewable status: 7 of 10 files reviewed, 26 unresolved discussions (waiting on @PlagueHO)

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 766 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Totally minor nitpick:

Can you indent the lines in between the <# #> - and through out:
e.g.
<#
    Line 1
    Line 2
#>

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 789 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use [System.Int32] here (trying to be consistent on not using Type Accelerators)

Done.

Modules/ComputerManagementDsc/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 823 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you remove blank line?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 57 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you change to [System.Guid]?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 64 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you correct the indenting of this line (and the followingones)

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 66 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you change to [System.Guid]?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 73 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you remove this blank line?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 76 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you change to [System.String]?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 101 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

It looks like the indenting is incorrect here.

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 113 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you convert to [System.String]

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 148 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you add a blank line here? Also it looks like there is a problem with the indenting of the whole block.

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 163 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Looks like there is an indenting issue.

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 169 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you remove blank line?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 171 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you remove blank line?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 174 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Change to [System.String]

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 187 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Change to [System.String]

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 193 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

It is probably a good idea to check that Set-ActivePowerPlan was called with expected parameters here.

Yes, good idea.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 206 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you remove blank line here?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 209 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you change to [System.String]?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 230 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you change to [System.Guid]- and throughout?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 237 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

It looks like indenting is wrong here.

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 247 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you remove blank line?

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 250 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Change to [System.String]

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 271 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Indenting issue here.

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 281 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Remove blank line.

Done.

Tests/Unit/MSFT_PowerPlan.Tests.ps1, line 284 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Change to [System.String]

Done.

PlagueHO

Reviewed 1 of 4 files at r4, 2 of 2 files at r5.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @J0F3)

Tests/Unit/ComputerManagementDsc.Common.Tests.ps1, line 698 at r5 (raw file):

                Mock `
                    -CommandName Get-PowerPlanUsingPInvoke `
                    -MockWith {

One thing you could do here is create a $mockBalancedPowerPlan variable and use that throughout as it will reduce duplication.

Eg.

$mockBalancedPowerPlan = {
    @{
       FriendlyName = 'Balanced'
        Guid         = [System.Guid]'381b4222-f694-41f0-9685-ff5bb260df2e'
    }
}

You could also do this in other places where you've got the same mock block being returned. I also tend to do this with ParamterFilter blocks when required.

Tests/Unit/ComputerManagementDsc.Common.Tests.ps1, line 805 at r5 (raw file):

                    $result.guid | Should -Be '381b4222-f694-41f0-9685-ff5bb260df2e'
                }

Can you remove these two blank lines?

J0F3

Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @PlagueHO)

Tests/Unit/ComputerManagementDsc.Common.Tests.ps1, line 698 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

One thing you could do here is create a $mockBalancedPowerPlan variable and use that throughout as it will reduce duplication.

Eg.
$mockBalancedPowerPlan = {
    @{
       FriendlyName = 'Balanced'
        Guid         = [System.Guid]'381b4222-f694-41f0-9685-ff5bb260df2e'
    }
}
You could also do this in other places where you've got the same mock block being returned. I also tend to do this with ParamterFilter blocks when required.

ok, makes sense 👍

Tests/Unit/ComputerManagementDsc.Common.Tests.ps1, line 805 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you remove these two blank lines?

Done.

PlagueHO

Awesome stuff @J0F3

Reviewed 1 of 1 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved

J0F3 added 13 commits January 11, 2019 19:44

Changes to use powercfg.exe instead of WMI/CIM

92c03dc

Update tests

bb6219a

Update changelog

ba8c22b

Fix markdown errors in changelog

a882d9b

Remove -PesterMaximumVersion 4.0.8 from appveyor.yml

87c9843

Add testcase for Set function when plan does not exist

e062889

Add Get-PowerPlan (replaces Get-PowerPlans) and Set-PowerPlan

533e7b3

Add functions to quey and set power plans with native Windows APIs

34c42d9

Update resource for new Get-ActivePowerPlan and Set-ActivePowerPlan f…

131bafd

…unctions

Update unit tests for resouce

1da0089

Remove obsolete unit tests

d6ff286

Update typenames to prevent conflicts

069714c

Update changelog

6f2eb52

J0F3 mentioned this pull request Jan 24, 2019

PowerPlan: Powercfg.exe instead of WMI and allow GUID as power plan name #202

Merged

7 tasks

PlagueHO added the needs review The pull request needs a code review. label Jan 25, 2019

PlagueHO self-requested a review January 25, 2019 09:31

Merge branch 'dev' into Windows-API-instead-WMI

8a2aa25

Merge branch 'dev' into Windows-API-instead-WMI

771979c

PlagueHO requested changes Mar 9, 2019

View reviewed changes

J0F3 added 2 commits March 9, 2019 22:24

improve formating based on review

30de7d9

Add new function for enumerating power plans

84808b9

J0F3 commented Mar 10, 2019

View reviewed changes

PlagueHO requested changes Mar 16, 2019

View reviewed changes

Improve formating based on review

7095e77

J0F3 commented Mar 23, 2019

View reviewed changes

PlagueHO requested changes Mar 23, 2019

View reviewed changes

move moked powerplans to reusable variables

96360a4

J0F3 commented Mar 24, 2019

View reviewed changes

PlagueHO approved these changes Mar 28, 2019

View reviewed changes

PlagueHO merged commit 0bfea1c into dsccommunity:dev Mar 28, 2019

kwirkykat removed the needs review The pull request needs a code review. label Mar 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PowerPlan: Windows APIs instead of WMI (alternative for PR #202) #203

PowerPlan: Windows APIs instead of WMI (alternative for PR #202) #203

J0F3 commented Jan 24, 2019 •

edited by joeyaiello

Loading

J0F3 commented Jan 24, 2019

PlagueHO commented Jan 25, 2019

PlagueHO commented Feb 16, 2019

PlagueHO commented Mar 1, 2019

codecov-io commented Mar 4, 2019 •

edited

Loading

J0F3 commented Mar 4, 2019

PlagueHO commented Mar 5, 2019

J0F3 commented Mar 6, 2019

PlagueHO left a comment

J0F3 left a comment

PlagueHO left a comment

J0F3 commented Mar 16, 2019

PlagueHO commented Mar 16, 2019

craigeaw commented Mar 22, 2019

PlagueHO commented Mar 22, 2019

J0F3 left a comment

PlagueHO left a comment

J0F3 left a comment

PlagueHO left a comment

PowerPlan: Windows APIs instead of WMI (alternative for PR #202) #203

PowerPlan: Windows APIs instead of WMI (alternative for PR #202) #203

Conversation

J0F3 commented Jan 24, 2019 • edited by joeyaiello Loading

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Task list

J0F3 commented Jan 24, 2019

PlagueHO commented Jan 25, 2019

PlagueHO commented Feb 16, 2019

PlagueHO commented Mar 1, 2019

codecov-io commented Mar 4, 2019 • edited Loading

Codecov Report

J0F3 commented Mar 4, 2019

PlagueHO commented Mar 5, 2019

J0F3 commented Mar 6, 2019

PlagueHO left a comment

Choose a reason for hiding this comment

J0F3 left a comment

Choose a reason for hiding this comment

PlagueHO left a comment

Choose a reason for hiding this comment

J0F3 commented Mar 16, 2019

PlagueHO commented Mar 16, 2019

craigeaw commented Mar 22, 2019

PlagueHO commented Mar 22, 2019

J0F3 left a comment

Choose a reason for hiding this comment

PlagueHO left a comment

Choose a reason for hiding this comment

J0F3 left a comment

Choose a reason for hiding this comment

PlagueHO left a comment

Choose a reason for hiding this comment

J0F3 commented Jan 24, 2019 •

edited by joeyaiello

Loading

codecov-io commented Mar 4, 2019 •

edited

Loading