NetBios: fixes #434, added two functions to common. #479

phbits · 2020-10-27T20:37:07Z

Pull Request (PR) description

Restructured GET and SET functions based on the following:

An adapter can only be configured via WMI method if IPEnabled=$true. Otherwise the registry key must be configured. This satisfies disabled and disconnected states.
Current setting can only be obtained via the registry since WMI is unreliable due to various adapter states.
Only configure an adapter when not in the desired state.
Retain wildcard support.

This Pull Request (PR) fixes the following issues

Fixes netbios: disabled adapters #434

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).
Resource documentation added/updated in README.md in 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.
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

PlagueHO · 2020-11-07T06:29:24Z

Hi @phbits - thanks for submitting this - there are a few style issues causing the HQRM failures. I can identify them for you if you like? Sorry for taking a long time to get onto these - I'm completely snowed under with day job till Dec. But I'll be back across all this then.

PlagueHO · 2020-12-01T07:39:44Z

Hi @phbits - I'm back on board now and have some capacity to work on DSC again. Are you planning to work on this? There are a few style and other issues that are causing this to fail - do you want me to do the review and identify key issues?

phbits · 2020-12-02T19:50:03Z

Hi @PlagueHO! I'm happy to continue working on this. The HQRM errors about a new line after an opening brace weren't making any sense to me so I put it on hold to learn Pester as the Unit tests will need to be fixed due to this PR. Let me take another look at the HQRM requirements and I'll update this thread.

PlagueHO

Hi @phbits - recommend using VSCode as your editor and enable the PS extension. This will automatically highlight many HQRM violations with underlines. Much easier that trying to pick them up other ways. I've identified some for you here.

Recommend reviewing style guidelines here: https://dsccommunity.org/styleguidelines/

Reviewed 1 of 3 files at r2.
Reviewable status: 1 of 3 files reviewed, 23 unresolved discussions (waiting on @phbits and @PlagueHO)

CHANGELOG.md, line 13 at r2 (raw file):

- NetBios
  - Fixes [Issue #434](https://github.com/dsccommunity/NetworkingDsc/issues/434).

Can you describe the fix?

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 86 at r2 (raw file):

        resource is not in the desired state.
    #>
    if ( $netAdapter -is [Array] )

HQRM:

if ($netAdapter -is [System.Array])

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 88 at r2 (raw file):

    if ( $netAdapter -is [Array] )
    {
        [string[]] $SettingResults = @()

Can you use [System.String[]]?

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 90 at r2 (raw file):

        [string[]] $SettingResults = @()

        foreach ( $netAdapterItem in $netAdapter )

HQRM:

foreach ($netAdapterItem in $netAdapter)

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 94 at r2 (raw file):

            $SettingResults += Get-NetAdapterNetbiosOptionsFromRegistry -NetworkAdapterGUID $netAdapterItem.GUID -Setting $Setting

            Write-Verbose -Message ($script:localizedData.CurrentNetBiosSettingMessage -f $netAdapterItem.NetConnectionID,$SettingResults[-1])

Can you add space before comma

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

        }

        [string[]] $WrongSettings = $SettingResults | Where-Object{ $_ -ne $Setting }

HQRM:

$wrongSettings = $SettingResults | Where-Object -FilterScript {
    $_ -ne $Setting
}

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 99 at r2 (raw file):

        [string[]] $WrongSettings = $SettingResults | Where-Object{ $_ -ne $Setting }

        if ([System.String]::IsNullOrEmpty($WrongSettings) -eq $false)

Can use:

if (-not [System.String]::IsNullOrEmpty($wrongSettings))

Should also be lowercase variable name.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 101 at r2 (raw file):

        if ([System.String]::IsNullOrEmpty($WrongSettings) -eq $false)
        {
            [string] $Setting = $WrongSettings[0]

No need to specify type.
Should be lower case variable name.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 161 at r2 (raw file):

    }

    if ( $netAdapter -is [Array] )

HQRM style issues. Don't use type accelerators.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 163 at r2 (raw file):

    if ( $netAdapter -is [Array] )
    {
        foreach ( $netAdapterItem in $netAdapter )

HQRM style issues.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 165 at r2 (raw file):

        foreach ( $netAdapterItem in $netAdapter )
        {
            $CurrentValue = Get-NetAdapterNetbiosOptionsFromRegistry -NetworkAdapterGUID $netAdapterItem.GUID -Setting $Setting

Variable name should start with lower case.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 168 at r2 (raw file):

            # Only make changes if necessary
            if ( $CurrentValue -ne $Setting )

HQRM style issues.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 232 at r2 (raw file):

<#
.SYNOPSIS
    Returns the NetbiosOptions value for a network adapter.

Blank line after this one.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 236 at r2 (raw file):

    Most reliable method of getting this value since network adapters
    can be in any number of states (e.g. disabled, disconnected)
    which can cause Win32 classes to not report the value.

Blank line after this one.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 238 at r2 (raw file):

    which can cause Win32 classes to not report the value.
.PARAMETER NetworkAdapterGUID
    Network Adapter GUID

Blank line after this one.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 249 at r2 (raw file):

    param
    (
        [Parameter(Mandatory=$true)]

[Parameter(Mandatory = $true)]

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

        [ValidatePattern("^\{[a-zA-Z0-9]{8}\-[a-zA-Z0-9]{4}\-[a-zA-Z0-9]{4}\-[a-zA-Z0-9]{4}\-[a-zA-Z0-9]{12}\}$")]
        [System.String]
        # Network Adapter GUID

Remove this line.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 254 at r2 (raw file):

        # Network Adapter GUID
        $NetworkAdapterGUID
        ,

Coma should be on previous line.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 262 at r2 (raw file):

    # Changing ErrorActionPreference variable since the switch -ErrorAction isn't supported.
    $CurrentErrorActionPreference = $ErrorActionPreference

Local variables should star with lower case.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 263 at r2 (raw file):

    # Changing ErrorActionPreference variable since the switch -ErrorAction isn't supported.
    $CurrentErrorActionPreference = $ErrorActionPreference
    $ErrorActionPreference        = 'SilentlyContinue'

Correct spacing.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 277 at r2 (raw file):

    switch ( $RegistryNetbiosOptions )
    {
        0 {       return 'Default' }

Correct spacing and apply HQRM styling:

0
{
    return 'Default'
}

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 282 at r2 (raw file):

        default {
            # Unknown value. Returning invalid setting to trigger Set-TargetResource
            [string[]] $InvalidSetting = 'Default','Enable','Disable' | Where-Object{ $_ -ne $Setting }

HQRM styling issues.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 289 at r2 (raw file):

} # end function Get-NetAdapterNetbiosOptionsFromRegistry

<#

Apply previous style corrections.

phbits · 2020-12-09T15:26:21Z

@PlagueHO - Thanks for the thorough review and feedback! I finished the unit tests yesterday and all the tests are completing successfully. Will start working on your suggested HQRM edits.

phbits

Reviewable status: 0 of 4 files reviewed, 23 unresolved discussions (waiting on @PlagueHO)

CHANGELOG.md, line 13 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you describe the fix?

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 86 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

HQRM:

if ($netAdapter -is [System.Array])

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 88 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use [System.String[]]?

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 90 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

HQRM:

foreach ($netAdapterItem in $netAdapter)

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 94 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add space before comma

Figured you meant a space after comma.

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

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

HQRM:

$wrongSettings = $SettingResults | Where-Object -FilterScript {
    $_ -ne $Setting
}

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 99 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can use:
if (-not [System.String]::IsNullOrEmpty($wrongSettings))
Should also be lowercase variable name.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 101 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

No need to specify type.
Should be lower case variable name.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 161 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

HQRM style issues. Don't use type accelerators.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 163 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

HQRM style issues.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 165 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Variable name should start with lower case.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 168 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

HQRM style issues.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 232 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Blank line after this one.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 236 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Blank line after this one.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 238 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Blank line after this one.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 249 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

[Parameter(Mandatory = $true)]

Done.

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

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Remove this line.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 254 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Coma should be on previous line.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 262 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Local variables should star with lower case.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 263 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Correct spacing.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 277 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Correct spacing and apply HQRM styling:
0
{
    return 'Default'
}

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 282 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

HQRM styling issues.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 289 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Apply previous style corrections.

Done.

phbits · 2020-12-10T04:01:59Z

@PlagueHO - All the tests are completing successful for this PR so I'm going to work on the DnsServerAddress PR. The updated unit tests will probably need to be included in the Reviewable app. You'll see that I included a number of additional tests especially with Test-TargetResource. First time using Pester and pleasantly surprised at how much value it adds.

PlagueHO

Reviewed 1 of 3 files at r2, 1 of 1 files at r3, 2 of 2 files at r4.
Reviewable status: all files reviewed, 17 unresolved discussions (waiting on @phbits)

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 97 at r4 (raw file):

        }

        [System.String[]] $wrongSettings = $settingResults | Where-Object -FilterScript {

Looking at this, I don't think you need to define the type here either because it is implicit given that $settingResults is already a [System.String[]].

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 233 at r4 (raw file):

<#
.SYNOPSIS

Sorry, I missed this one - minor nitpick: can you indent all lines in this block to match previous comment blocks?

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 270 at r4 (raw file):

    $registryNetbiosOptions = Get-ItemPropertyValue -Name 'NetbiosOptions' `
                    -Path "HKLM:\SYSTEM\CurrentControlSet\Services\NetBT\Parameters\Interfaces\Tcpip_$($NetworkAdapterGUID)"

Can we add this path as a constant to the top of the module? E.g.

$hklmInterfacesPath = 'HKLM:\SYSTEM\CurrentControlSet\Services\NetBT\Parameters\Interfaces\'

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 285 at r4 (raw file):

            return 'Default'
        }
        1

Can add blank line above this one.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 289 at r4 (raw file):

            return 'Enable'
        }
        2

Can add blank line above this one.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 296 at r4 (raw file):

        {
            # Unknown value. Returning invalid setting to trigger Set-TargetResource
            [string[]] $invalidSetting = 'Default','Enable','Disable' | Where-Object -FilterScript {

You can remove the type assignment as it is already implicitly a [System.String[]].

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 306 at r4 (raw file):

<#
.SYNOPSIS

Can you indent all lines in comment block?

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 333 at r4 (raw file):

        [Parameter(Mandatory = $true)]
        [System.Object]
        $NetworkAdapterObj,

Nitpick: Could this parameter name not include Obj? Alternatively expand Obj to Object.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 345 at r4 (raw file):

    )

    Write-Verbose -Message ($script:localizedData.SetNetBiosMessage -f $InterfaceAlias,$Setting)

Can you add space after comma? Suggest using VSCode autoformatting - it will fix all these kinds of minor style issues: https://dscottraynsford.wordpress.com/2017/11/18/auto-formatting-powershell-in-visual-studio-code/

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 367 at r4 (raw file):

    {
        <#
        IPEnabled=$false can only be configured via registry

Can you indent this line and the next?

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 371 at r4 (raw file):

        #>
        $setItemPropertyParameters = @{
            Path  = "HKLM:\SYSTEM\CurrentControlSet\Services\NetBT\Parameters\Interfaces\Tcpip_$($NetworkAdapterObj.SettingID)"

This could then use the constant defined earlier.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 377 at r4 (raw file):

        $null = Set-ItemProperty @setItemPropertyParameters
    }

Can remove this blank line.

tests/Unit/DSC_NetBios.Tests.ps1, line 190 at r4 (raw file):

                        Mock -CommandName Get-CimInstance -MockWith $script:mockNetadapterA
                        Mock -CommandName Get-ItemPropertyValue -MockWith { return $testCase.SettingInt } `
                                                                -ParameterFilter $script:getItemPropertyValue_NetbiosOptions_One_ParameterFilter

Can you reduce indent on this line to prevent wrap?

tests/Unit/DSC_NetBios.Tests.ps1, line 191 at r4 (raw file):

                        Mock -CommandName Get-ItemPropertyValue -MockWith { return $testCase.SettingInt } `
                                                                -ParameterFilter $script:getItemPropertyValue_NetbiosOptions_One_ParameterFilter

Can you trim white space on this line?

tests/Unit/DSC_NetBios.Tests.ps1, line 207 at r4 (raw file):

                        It 'Should call expected mocks' {
                            Assert-MockCalled -CommandName Get-CimInstance -ParameterFilter $script:getCimInstanceParameterFilter -Exactly -Times 1

Can you add back ticks to this line to reduce line length (we try to keep them under 120 to make reviews easier)

tests/Unit/DSC_NetBios.Tests.ps1, line 217 at r4 (raw file):

                    Context "When both NetBios over TCP/IP is set to 'Default' on both and Setting is 'Default'" {
                        Mock -CommandName Get-CimInstance -MockWith $script:mockNetadapterMulti
                        Mock -CommandName Get-ItemPropertyValue -MockWith { return 0 } -ParameterFilter $script:getItemPropertyValue_NetbiosOptions_One_ParameterFilter

Can you add back ticks to this line to reduce line length (we try to keep them under 120 to make reviews easier)

And throughout.

tests/Unit/DSC_NetBios.Tests.ps1, line 648 at r4 (raw file):

                        }
                    }
                }

Can you add some new tests to cover just the new functions Get-NetAdapterNetbiosOptionsFromRegistry and Get-SetAdapterNetbiosOptions?

PlagueHO

Reviewable status: all files reviewed, 18 unresolved discussions (waiting on @phbits)

CHANGELOG.md, line 13 at r4 (raw file):

- NetBios
  - Fixes configuring network adapters in a disconnected or disabled state. Fixes [Issue #434](https://github.com/dsccommunity/NetworkingDsc/issues/434).

Just for consistency can you use the format - Fixes configuring network adapters in a disconnected or disabled state - fixes [Issue #434](https://github.com/dsccommunity/NetworkingDsc/issues/434).

phbits

Reviewable status: 1 of 4 files reviewed, 18 unresolved discussions (waiting on @PlagueHO)

CHANGELOG.md, line 13 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Just for consistency can you use the format - Fixes configuring network adapters in a disconnected or disabled state - fixes [Issue #434](https://github.com/dsccommunity/NetworkingDsc/issues/434).

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 97 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Looking at this, I don't think you need to define the type here either because it is implicit given that $settingResults is already a [System.String[]].

It defaults to System.Object[] if not explicitly declared. Some time ago this one got the best of me and took far longer then I'd like to admit to finally identify.

PS C:\> [System.String[]] $settingResults = 'Enable','Disable','Default'
PS C:\> $wrongSettings = $settingResults | Where-Object -FilterScript { $_ -ne 'Enable' }
PS C:\> Get-Member -InputObject $wrongSettings | findstr 'TypeName'
   TypeName: System.Object[]
PS C:\> [System.String[]] $wrongSettings = $settingResults | Where-Object -FilterScript { $_ -ne 'Enable' }
PS C:\> Get-Member -InputObject $wrongSettings | findstr 'TypeName'
   TypeName: System.String[]

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 233 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Sorry, I missed this one - minor nitpick: can you indent all lines in this block to match previous comment blocks?

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 270 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can we add this path as a constant to the top of the module? E.g.
$hklmInterfacesPath = 'HKLM:\SYSTEM\CurrentControlSet\Services\NetBT\Parameters\Interfaces\'

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 285 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can add blank line above this one.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 289 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can add blank line above this one.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 296 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

You can remove the type assignment as it is already implicitly a [System.String[]].

Doh! Missed setting this to [System.String[]]. Leaving the explicit assignment as mentioned above.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 306 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you indent all lines in comment block?

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 333 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: Could this parameter name not include Obj? Alternatively expand Obj to Object.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 345 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add space after comma? Suggest using VSCode autoformatting - it will fix all these kinds of minor style issues: https://dscottraynsford.wordpress.com/2017/11/18/auto-formatting-powershell-in-visual-studio-code/

Done. Thanks for the reference. Much appreciated!

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 367 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you indent this line and the next?

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 371 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

This could then use the constant defined earlier.

Done.

source/DSCResources/DSC_NetBios/DSC_NetBios.psm1, line 377 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can remove this blank line.

Done.

tests/Unit/DSC_NetBios.Tests.ps1, line 190 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you reduce indent on this line to prevent wrap?

Done.

tests/Unit/DSC_NetBios.Tests.ps1, line 191 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you trim white space on this line?

Done. Double-checked and I have render whitespace on.

tests/Unit/DSC_NetBios.Tests.ps1, line 207 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add back ticks to this line to reduce line length (we try to keep them under 120 to make reviews easier)

Done.

tests/Unit/DSC_NetBios.Tests.ps1, line 217 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add back ticks to this line to reduce line length (we try to keep them under 120 to make reviews easier)

And throughout.

Updated DSC_NetBios.Tests.ps1 to satisfy 120.

tests/Unit/DSC_NetBios.Tests.ps1, line 648 at r4 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add some new tests to cover just the new functions Get-NetAdapterNetbiosOptionsFromRegistry and Get-SetAdapterNetbiosOptions?

Done. Also led me to fix issues in prior written tests.

PlagueHO · 2020-12-26T08:22:55Z

Hi @phbits - can you resolve the conflicts on this one? Check that the changes in the CHANGELOG.md appear in the [Unreleased] section.

phbits

Updated CHANGELOG.md.

Reviewable status: 1 of 4 files reviewed, 18 unresolved discussions (waiting on @PlagueHO)

PlagueHO · 2020-12-30T05:47:30Z

Hi @phbits - looks like there are still conflicts in the CHANGELOG.md

phbits · 2020-12-30T16:39:14Z

@PlagueHO , I resolved the CHANGELOG conflict. Will resolve the DnsServerAddress PR after this PR is closed.

PlagueHO

Reviewed 2 of 3 files at r5, 1 of 1 files at r7.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @phbits)

tests/Unit/DSC_NetBios.Tests.ps1, line 858 at r7 (raw file):

            }
        }
        Describe 'DSC_NetBios\Get-NetAdapterNetbiosOptionsFromRegistry' {

Some final style nitpicks (sorry 😁 - I know these are a bit of a pain) - can you a blank line before each Describe and Context block?

Added blank line before each Describe and Context block

phbits

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @PlagueHO)

tests/Unit/DSC_NetBios.Tests.ps1, line 858 at r7 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Some final style nitpicks (sorry 😁 - I know these are a bit of a pain) - can you a blank line before each Describe and Context block?

NP, blank lines added. Happy New Year! 🎉

PlagueHO

I am sorry for how long this took.

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

phbits added 3 commits October 27, 2020 20:21

fixes #434, added two functions to common.

47d9636

HQRM adds, moved the 2 new common functions to DSC_NetBios.psm1

3d71023

HQRM edits

4130e31

PlagueHO added the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Dec 1, 2020

PlagueHO self-requested a review December 1, 2020 07:40

HQRM edits.

129a9cf

PlagueHO requested changes Dec 6, 2020

View reviewed changes

NetBios: updated unit tests

26a8f71

phbits added 2 commits December 9, 2020 16:40

Reviewable HQRM edits

a02424a

change to system.array

ec48daf

phbits commented Dec 9, 2020

View reviewed changes

PlagueHO added needs review The pull request needs a code review. and removed waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. labels Dec 12, 2020

PlagueHO requested changes Dec 12, 2020

View reviewed changes

PlagueHO added waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed needs review The pull request needs a code review. labels Dec 12, 2020

PlagueHO requested changes Dec 12, 2020

View reviewed changes

Reviewable edits

6146d55

phbits commented Dec 15, 2020

View reviewed changes

Update CHANGELOG.md

3943f92

phbits commented Dec 27, 2020

View reviewed changes

Merge branch 'master' into Netbios

36ccfee

PlagueHO requested changes Dec 31, 2020

View reviewed changes

phbits added 2 commits December 31, 2020 21:26

Reviewable edits.

7f3e015

Added blank line before each Describe and Context block

removed spaces

c4a429c

phbits commented Jan 1, 2021

View reviewed changes

PlagueHO approved these changes Feb 22, 2021

View reviewed changes

PlagueHO merged commit c829402 into dsccommunity:master Feb 22, 2021

phbits mentioned this pull request May 30, 2022

Netbios: Fails to disable when NIC has link down #511

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NetBios: fixes #434, added two functions to common. #479

NetBios: fixes #434, added two functions to common. #479

phbits commented Oct 27, 2020 •

edited

Loading

PlagueHO commented Nov 7, 2020

PlagueHO commented Dec 1, 2020

phbits commented Dec 2, 2020

PlagueHO left a comment

phbits commented Dec 9, 2020

phbits left a comment

phbits commented Dec 10, 2020 •

edited

Loading

PlagueHO left a comment

PlagueHO left a comment

phbits left a comment

PlagueHO commented Dec 26, 2020

phbits left a comment

PlagueHO commented Dec 30, 2020

phbits commented Dec 30, 2020

PlagueHO left a comment

phbits left a comment

PlagueHO left a comment

NetBios: fixes #434, added two functions to common. #479

NetBios: fixes #434, added two functions to common. #479

Conversation

phbits commented Oct 27, 2020 • edited Loading

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Task list

PlagueHO commented Nov 7, 2020

PlagueHO commented Dec 1, 2020

phbits commented Dec 2, 2020

PlagueHO left a comment

Choose a reason for hiding this comment

phbits commented Dec 9, 2020

phbits left a comment

Choose a reason for hiding this comment

phbits commented Dec 10, 2020 • edited Loading

PlagueHO left a comment

Choose a reason for hiding this comment

PlagueHO left a comment

Choose a reason for hiding this comment

phbits left a comment

Choose a reason for hiding this comment

PlagueHO commented Dec 26, 2020

phbits left a comment

Choose a reason for hiding this comment

PlagueHO commented Dec 30, 2020

phbits commented Dec 30, 2020

PlagueHO left a comment

Choose a reason for hiding this comment

phbits left a comment

Choose a reason for hiding this comment

PlagueHO left a comment

Choose a reason for hiding this comment

phbits commented Oct 27, 2020 •

edited

Loading

phbits commented Dec 10, 2020 •

edited

Loading