Skip to content

Added new DSC resource xVirtualMemory#64

Merged
mbreakey3 merged 32 commits intodsccommunity:devfrom
nyanhp:feature/xVirtualMemory
May 4, 2017
Merged

Added new DSC resource xVirtualMemory#64
mbreakey3 merged 32 commits intodsccommunity:devfrom
nyanhp:feature/xVirtualMemory

Conversation

@nyanhp
Copy link
Copy Markdown
Contributor

@nyanhp nyanhp commented Feb 27, 2017

Added a new DSC resource xVirtualMemory to be able to set the paging file on a machine. Fixes #63 . In addition to the new resource, unit and integration tests and a sample script were provided as well.


This change is Reviewable

@msftclas
Copy link
Copy Markdown

@nyanhp,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

… Testcase checking the hashtable keys will now check them after sorting them properly since the order they were returned in during the automated appveyor tests was different than on a local system.
@PlagueHO
Copy link
Copy Markdown
Member

PlagueHO commented Mar 1, 2017

Really great job @nyanhp ! Thank you for submitting this! Sorry about all the nitpicky style changes - hope it's not too annoying!


Reviewed 4 of 7 files at r1, 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 61 unresolved discussions.


README.md, line 81 at r2 (raw file):

 
## xVirtualMemory
xVirtualMemory resource is used to set the properties of the paging file on the local computer.

Please add CRLF above this line to meet Markdown rules.


README.md, line 83 at r2 (raw file):

xVirtualMemory resource is used to set the properties of the paging file on the local computer.
xVirtualMemory has the following properties:
* Type: The type of the paging settings, mandatory, out of "AutoManagePagingFile","CustomSize","SystemManagedSize","NoPagingFile"

Please add CRLF above this line to meet Markdown rules.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 16 at r2 (raw file):

    )

    Write-Verbose 'Getting current page file settings'

Can you please convert positional parameter to named parameter?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 27 at r2 (raw file):

    [bool] $isSystemManaged = (Get-CimInstance -ClassName Win32_ComputerSystem).AutomaticManagedPagefile
    
    if ($isSystemManaged) {

Move { to next line please.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 34 at r2 (raw file):

    $driveItem = [System.IO.DriveInfo] $Drive

    Write-Verbose "Pagefile was not automatically managed. Retrieving detailed page file settings with query Select * from Win32_PageFileSetting where SettingID='pagefile.sys @ $($driveItem.Name.Substring(0,2))'"

Can you please convert positional parameter to named parameter?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 37 at r2 (raw file):

    # Find existing page file settings by drive letter
    $virtualMemoryInstance =  Get-CimInstance -Namespace root\cimv2 -Query "Select * from Win32_PageFileSetting where SettingID='pagefile.sys @ $($driveItem.Name.Substring(0,2))'"

Remove extra space after =


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 39 at r2 (raw file):

    $virtualMemoryInstance =  Get-CimInstance -Namespace root\cimv2 -Query "Select * from Win32_PageFileSetting where SettingID='pagefile.sys @ $($driveItem.Name.Substring(0,2))'"
    
    if (-not $virtualMemoryInstance) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 44 at r2 (raw file):

    }

    if ($virtualMemoryInstance.InitialSize -eq 0 -and $virtualMemoryInstance.MaximumSize -eq 0) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 47 at r2 (raw file):

        $returnValue.Type = 'SystemManagedSize'
    }
    else {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 83 at r2 (raw file):

    $SystemInfo = Get-CimInstance -Class Win32_ComputerSystem

    switch($Type) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 84 at r2 (raw file):

    switch($Type) {
        "AutoManagePagingFile" {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 91 at r2 (raw file):

            } 

            Write-Verbose 'Enabling AutoManagePagingFile'

Can you please convert positional parameter to named parameter?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 97 at r2 (raw file):

            break
        }
        "CustomSize" {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 98 at r2 (raw file):

        }
        "CustomSize" {
            if ($SystemInfo.AutomaticManagedPageFile) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 107 at r2 (raw file):

                } 

                Write-Verbose 'Disabling AutoManagePagingFile'

Can you please convert positional parameter to named parameter?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 113 at r2 (raw file):

            $driveInfo = [System.IO.DriveInfo] $Drive
            if (-not $driveInfo.IsReady) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 119 at r2 (raw file):

            $pageFileName = Join-Path $driveInfo.Name 'pagefile.sys'

            Write-Verbose ('Checking if a paging file already exists at {0}' -f $pageFileName)

Can you please convert positional parameter to named parameter?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 121 at r2 (raw file):

            Write-Verbose ('Checking if a paging file already exists at {0}' -f $pageFileName)
            $existingPageFileSetting = Get-CimInstance -Namespace root\cimv2 -Query "Select * from Win32_PageFileSetting where SettingID='pagefile.sys @ $($driveInfo.Name.Substring(0,2))'"
            if (-not $existingPageFileSetting) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 126 at r2 (raw file):

            <# 
            # New-CimInstance does not support properties InitialSize and MaximumSize. Therefore, create

Nitpick - no need for #
And on next line


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 138 at r2 (raw file):

            } 

            Write-Verbose ("Setting page file to {0}. Initial size {1}MB, maximum size {2}MB" -f $pageFileName, $InitialSize, $MaximumSize)

Can you please convert positional parameter to named parameter?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 144 at r2 (raw file):

            break
        }
        "SystemManagedSize" {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 145 at r2 (raw file):

        }
        "SystemManagedSize" {
            if ($SystemInfo.AutomaticManagedPageFile) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 152 at r2 (raw file):

                } 

                Write-Verbose 'Disabling AutoManagePagingFile'

Can you please convert positional parameter to named parameter?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 158 at r2 (raw file):

            $driveInfo = [System.IO.DriveInfo] $Drive
            if (-not $driveInfo.IsReady) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 162 at r2 (raw file):

            }

            $pageFileName = Join-Path $driveInfo.Name 'pagefile.sys'

Can you please convert positional parameters to named parameters?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 167 at r2 (raw file):

            $existingPageFileSetting = Get-CimInstance -Namespace root\cimv2 -Query "Select * from Win32_PageFileSetting where SettingID='pagefile.sys @ $($driveInfo.Name.Substring(0,2))'"
            if (-not $existingPageFileSetting) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 181 at r2 (raw file):

            } 

            Write-Verbose "Enabling system-managed page file on $pageFileName"

Can you please convert positional parameter to named parameter?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 187 at r2 (raw file):

            break
        }
        "NoPagingFile" {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 188 at r2 (raw file):

        }
        "NoPagingFile" {
            if ($SystemInfo.AutomaticManagedPageFile) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 199 at r2 (raw file):

            $driveInfo = [System.IO.DriveInfo] $Drive
            if (-not $driveInfo.IsReady) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 206 at r2 (raw file):

                
            $existingPageFileSetting = Get-CimInstance -Namespace root\cimv2 -Query "Select * from Win32_PageFileSetting where SettingID='pagefile.sys @ $($driveInfo.Name.Substring(0,2))'"
            if ($existingPageFileSetting) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 207 at r2 (raw file):

            $existingPageFileSetting = Get-CimInstance -Namespace root\cimv2 -Query "Select * from Win32_PageFileSetting where SettingID='pagefile.sys @ $($driveInfo.Name.Substring(0,2))'"
            if ($existingPageFileSetting) {
                Write-Verbose "Removing existing page file $($existingPageFileSetting.Name)"

Can you please convert positional parameter to named parameter?


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 216 at r2 (raw file):

            break
        }
        default {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 248 at r2 (raw file):

    switch($Type) {
        "AutoManagePagingFile" {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 252 at r2 (raw file):

            break
        }
        "CustomSize" {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 253 at r2 (raw file):

        }
        "CustomSize" {
            if ($SystemInfo.AutomaticManagedPageFile) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 260 at r2 (raw file):

            $driveInfo = [System.IO.DriveInfo] $Drive
            $PageFile = Get-CimInstance -Class Win32_PageFileSetting -Filter "SettingID='pagefile.sys @ $($driveInfo.Name.Substring(0,2))'"
            if (-not $PageFile) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 265 at r2 (raw file):

            }

            if (-not ($PageFile.InitialSize -eq $InitialSize -and $PageFile.MaximumSize -eq $MaximumSize)) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 273 at r2 (raw file):

            break
        }
        "SystemManagedSize" {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 274 at r2 (raw file):

        }
        "SystemManagedSize" {
            if ($SystemInfo.AutomaticManagedPageFile) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 281 at r2 (raw file):

            $driveInfo = [System.IO.DriveInfo] $Drive
            $PageFile = Get-CimInstance -Class Win32_PageFileSetting -Filter "SettingID='pagefile.sys @ $($driveInfo.Name.Substring(0,2))'"
            if (-not $PageFile) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 286 at r2 (raw file):

            }

            if (-not ($PageFile.InitialSize -eq 0 -and $PageFile.MaximumSize -eq 0)) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 294 at r2 (raw file):

            break
        }
        "NoPagingFile" {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 295 at r2 (raw file):

        }
        "NoPagingFile" {
            if ($SystemInfo.AutomaticManagedPageFile) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 303 at r2 (raw file):

            $PageFile = Get-CimInstance -Class Win32_PageFileSetting -Filter "SettingID='pagefile.sys @ $($driveInfo.Name.Substring(0,2))'"
                
            if ($PageFile) {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 311 at r2 (raw file):

            break
        }
        default {

Please move { to next line.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.schema.mof, line 5 at r2 (raw file):

class MSFT_xVirtualMemory : OMI_BaseResource
{
    [Key] String Drive;

Can you please add description values to each field? This will help when we move over to auto-documentation generation from the MOF files (being worked on at the moment).


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 13 at r2 (raw file):

   Code in HEADER, FOOTER and DEFAULT TEST regions are standard and may be moved into
   DSCResource.Tools in Future and therefore should not be altered if possible.
#>

Can you please delete the header comment?


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 46 at r2 (raw file):

        Context "Set page file to automatically managed" {
            $CurrentConfig = "setToAuto"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)

Can you please convert positional parameters to named parameters?


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 47 at r2 (raw file):

            $CurrentConfig = "setToAuto"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)
            $ConfigMof = (Join-Path $ConfigDir "localhost.mof")

Can you please convert positional parameters to named parameters?


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 68 at r2 (raw file):

        Context "Set page file to custom size" {
            $CurrentConfig = "setToCustom"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)

Can you please convert positional parameters to named parameters?


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 69 at r2 (raw file):

            $CurrentConfig = "setToCustom"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)
            $ConfigMof = (Join-Path $ConfigDir "localhost.mof")

Can you please convert positional parameters to named parameters?


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 90 at r2 (raw file):

        Context "Set page file to system managed" {
            $CurrentConfig = "setToSystemManaged"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)

Can you please convert positional parameters to named parameters?


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 91 at r2 (raw file):

            $CurrentConfig = "setToSystemManaged"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)
            $ConfigMof = (Join-Path $ConfigDir "localhost.mof")

Can you please convert positional parameters to named parameters?


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 112 at r2 (raw file):

        Context "Set page file to none" {
            $CurrentConfig = "setToNone"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)

Can you please convert positional parameters to named parameters?


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 113 at r2 (raw file):

            $CurrentConfig = "setToNone"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)
            $ConfigMof = (Join-Path $ConfigDir "localhost.mof")

Can you please convert positional parameters to named parameters?


Tests/Integration/MSFT_xVirtualMemory.Integration.Tests.ps1, line 141 at r2 (raw file):

    #endregion

    # TODO: Other Optional Cleanup Code Goes Here...

Can you delete this line?


Tests/Unit/MSFT_xComputer.Tests.ps1, line 133 at r2 (raw file):

                    $Result = Get-TargetResource -Name $env:COMPUTERNAME
                    $Result.GetType().Fullname | Should Be 'System.Collections.Hashtable'
                    $Result.Keys | Sort-Object | Should Be @('Credential', 'CurrentOU', 'DomainName', 'JoinOU', 'Name',  'UnjoinCredential', 'WorkGroupName')

Is this an intentional fix to the tests? If so, no worries - I can see it is correct 😁


Tests/Unit/MSFT_xVirtualMemory.Tests.ps1, line 46 at r2 (raw file):

                BeforeEach {
                    Mock -CommandName Get-CimInstance -MockWith {
                        return New-Object Object | 

Nitpick: Why not:

[PSObject] @{ AutomaticManagegPageFile = $false; Name = 'D:\pagefile.sys' }

Also, if the Mock return is going to be used more than once (as is the case further down), you can store the object in a variable and return that from the mock to reduce code.


Tests/Unit/MSFT_xVirtualMemory.Tests.ps1, line 125 at r2 (raw file):

                    MaximumSize = 1337
                } 
                It 'Should throw if no valid drive letter has been used' { { Set-TargetResource @testParameters } | Should Throw

Nitpick: Move inner block to it's own line to improve readability.


Tests/Unit/MSFT_xVirtualMemory.Tests.ps1, line 134 at r2 (raw file):

                    MaximumSize = 1337
                } 
                It 'Should throw if the drive is not ready' { { Set-TargetResource @testParameters } | Should Throw

Nitpick: Move inner block to it's own line to improve readability.


Comments from Reviewable

@nyanhp
Copy link
Copy Markdown
Contributor Author

nyanhp commented Mar 1, 2017

No worries @PlagueHO , a consistent style is very important! In Visual Studio with Style Cop it was easier though :) I fixed all styling issues and pushed my changes. Thank you for reviewing them!

Regarding the modified test for xComputer: Somehow when AppVeyor kicked in, the order of the results was checked very strictly. On my client, I had no problems with the test as it was before. Since my tests kept failing due to this, I changed the expected order and the order of the returned values to be alphabetic.

@nyanhp
Copy link
Copy Markdown
Contributor Author

nyanhp commented Mar 14, 2017

Hi @PlagueHO , is there anything else I need to do? I fixed everything and pushed my code. Do I need to do things on Reviewable as well?

@msftclas
Copy link
Copy Markdown

@nyanhp, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@PlagueHO
Copy link
Copy Markdown
Member

Hi @nyanhp, no, you're good. Leave it with me and ill get a review done tonight after work! Really appreciate your work and patience! Sorry about the delay!

@PlagueHO
Copy link
Copy Markdown
Member

Hi @nyanhp - looking really good - just a few changes missed ( moving { to next line ). Will be good to go after those are fixed.


Reviewed 5 of 5 files at r3.
Review status: all files reviewed at latest revision, 14 unresolved discussions.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 248 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 252 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 253 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 260 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 265 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 273 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 274 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 281 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 286 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 294 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 295 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 303 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 311 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please move { to next line.

Missed this one.


Comments from Reviewable

@nyanhp
Copy link
Copy Markdown
Contributor Author

nyanhp commented Mar 22, 2017

Don't worry @PlagueHO . I was just asking since this was my first PR and I was not entirely sure if I had to do additional things. I think I formatted everything I missed before.

@msftclas
Copy link
Copy Markdown

@nyanhp, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@PlagueHO
Copy link
Copy Markdown
Member

Great job @nyanhp! Sorry, I missed a couple of issues (each function must have Comment Based help) though.


Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 1 at r4 (raw file):

function Get-TargetResource

Doh! Something I nearly missed: All functions need to have Comment based help (see https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#functions-have-comment-based-help)


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 63 at r4 (raw file):

}

function Set-TargetResource

Doh! Something I nearly missed: All functions need to have Comment based help (see https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#functions-have-comment-based-help)


DSCResources/MSFT_xVirtualMemory/MSFT_xVirtualMemory.psm1, line 242 at r4 (raw file):

}

function Test-TargetResource

Doh! Something I nearly missed: All functions need to have Comment based help (see https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#functions-have-comment-based-help)


Comments from Reviewable

@nyanhp
Copy link
Copy Markdown
Contributor Author

nyanhp commented Apr 21, 2017

Hi @PlagueHO, done.
Out of curiosity: Why don't the other Get/Set/Test functions in the xComputerManagement resources have comment-based help? I can open a new issue for that and add the help.

@PlagueHO
Copy link
Copy Markdown
Member

PlagueHO commented May 4, 2017

Hi @nyanhp -

Regards the comment based help:

over time the style guidelines have evolved somewhat - so earlier modules might not meet the current standards. This has introduced tech debt that we'd like to fix (but it takes time) - I'm slowly working my way through it (working on xCertificate ATM). We've also been trying to move the core resource kit modules over to meet HQRM standards (https://github.com/PowerShell/DscResources/blob/master/HighQualityModuleGuidelines.md). Comment Based help is one of the HQRM standards.

I will say that there has been a bit of too and fro over this particular one in the past and it was decided to keep it 😁

Long story short: We'll want to add comment based help to any modules that are missing it.

I'll finish this review over the weekend (been a bit of a hectic week). Thanks for all your help and patience!

@PlagueHO
Copy link
Copy Markdown
Member

PlagueHO commented May 4, 2017

Actually, it wasn't a big review - so I did it now.

:lgtm:


Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@PlagueHO
Copy link
Copy Markdown
Member

PlagueHO commented May 4, 2017

@mbreakey3 - if you get a chance, could you merge this one for us? I think it's good to go. Cheers Dan

@mbreakey3 mbreakey3 merged commit a554214 into dsccommunity:dev May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants