BREAKING CHANGE: Changes to xSQLServerMemory

Changes to xSQLServerMemory

BREAKING CHANGE: The mandatory parameter now include SQLInstanceName
Added Pester Test

This Pull Request (PR) fixes the following issues:
Fixes #293

[x] Change details added to Unreleased section of CHANGELOG.md?
[x] Added/updated documentation, comment-based help and descriptions in .schema.mof files where appropriate?
[x] Examples appropriately updated?
[x] New/changed code adheres to Style Guidelines?
[x] Unit and (optional) Integration tests created/updated where possible?

This change is

Pester in development.
Could you start reviewing ?
I will add the unit test in couple of days ;)

@luigilink I can start review without the tests ready, but I need to review other before this one. I will get to this eventually. :)

@luigilink If you could use the template https://github.com/PowerShell/DscResources/blob/master/Tests.Template/unit_template.ps1 as the baseline for any further tests that would be great :)

Reviewed 1 of 7 files at r2.
Review status: 1 of 9 files reviewed at latest revision, 1 unresolved discussion.

xSQLServerHelper.psm1, line 1743 at r2 (raw file):

    This cmdlet is used to return the Dynamic MaxMemory of a SQL Instance
#>
function Get-SqlDscDynamicMaxMemory

Could you please move this function to the MSFT_xSQLServerMemory.psm1 file instead. You can add it to the end of the file.
I think we should make xSQLServerHelper to only contain helper functions that are used by two or more resources.
Also, make sure to test this code in MSFT_xSQLServerMemory.Tests.ps1. Either as part of the present tests or as a separate test.

Comments from Reviewable

@luigilink Can you please see if there is anything in the abandoned PR #122 that you could incorporate here? Or should we close PR #122 if it is obsolete?

@johlju Yes you can close PR#122, I took some code improvement.
But I re-developp the Get-SqlDscDynamicMaxMemory function :)

Ready for review 😄

Review status: 0 of 8 files reviewed at latest revision, 1 unresolved discussion.

xSQLServerHelper.psm1, line 1743 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Could you please move this function to the MSFT_xSQLServerMemory.psm1 file instead. You can add it to the end of the file.
I think we should make xSQLServerHelper to only contain helper functions that are used by two or more resources.
Also, make sure to test this code in MSFT_xSQLServerMemory.Tests.ps1. Either as part of the present tests or as a separate test.

Done.

Comments from Reviewable

Reviewed 2 of 8 files at r1, 4 of 7 files at r2, 2 of 3 files at r3.
Review status: 7 of 8 files reviewed at latest revision, 93 unresolved discussions.

CHANGELOG.md, line 3 at r3 (raw file):

# Change log for xSQLServer

## Unreleased

Please rebase and make sure your changes stays under the Unreleased section, and does not get moved to the <IP_ADDRESS> version.

CHANGELOG.md, line 74 at r3 (raw file):

    - Get-SqlDatabaseRecoveryModel
    - Set-SqlDatabaseRecoveryModel
    - Get-SqlDscDynamicMaxMemory

This can be removed

CHANGELOG.md, line 116 at r3 (raw file):

  - Now Restart-SqlService will correctly show the services it restarts. Also fixed PSSA warnings.
- Changes to xSQLServerMemory
  - BREAKING CHANGE: The mandatory parameters now include SQLServer, and SQLInstanceName.

Please remove SQLServer frpn the mandatory parameters.
Please add that DynamicAlloc is no longer mandatory.

README.md, line 591 at r3 (raw file):

This resource set the min server memory and max server memory configuration option.

This resource sets the min server memory and max server memory configuration option. That means it sets the minimum and the maximum amount of memory, in MB, in the buffer pool used by the instance of SQL Server.

README.md, line 592 at r3 (raw file):

This resource set the min server memory and max server memory configuration option.
The default setting for min server memory is 0, and the default setting for max server memory is<PHONE_NUMBER> MB.

Change 'min' to 'minimum'
Change 'max' to 'maximum'

README.md, line 593 at r3 (raw file):

This resource set the min server memory and max server memory configuration option.
The default setting for min server memory is 0, and the default setting for max server memory is<PHONE_NUMBER> MB.
Read more about  min server memory and max server memory in this article [Server Memory Server Configuration Options](https://msdn.microsoft.com/en-us/library/ms178067.aspx).

Change 'min' to 'minimum'.  Also a two space before, change to one space.
Change 'max' to 'maximum'

README.md, line 595 at r3 (raw file):

Read more about  min server memory and max server memory in this article [Server Memory Server Configuration Options](https://msdn.microsoft.com/en-us/library/ms178067.aspx).

#### Formula for dynamically allocating max memory

This way of dynamically setting the values, is that a best practice. Is there an article you can link to about it? If not, can you add a text to say how you come by to using this particular formula?

README.md, line 597 at r3 (raw file):

(NumOfSQLThreads x ThreadStackSize)

Change to:
(NumOfSQLThreads * ThreadStackSize)

README.md, line 597 at r3 (raw file):

#### Formula for dynamically allocating max memory

* SQL Max Memory = TotalPhyMem - (NumOfSQLThreads x ThreadStackSize) - (1GB x CEILING(NumOfCores/4)) - OS Reserved.

Could you write the formula in more clear text, like you did for the MaxDop PR? As of now it is kind of hard to read it.

README.md, line 610 at r3 (raw file):

* **[String] SQLInstance** _(Key)_: The name of the SQL instance to be configured.
* **[String] SQLServer** _(Required)_: The host name of the SQL Server to be configured.

Change to type qualifier Write.

README.md, line 611 at r3 (raw file):

* **[String] SQLInstance** _(Key)_: The name of the SQL instance to be configured.
* **[String] SQLServer** _(Required)_: The host name of the SQL Server to be configured.
* **[Boolean] DyamicAlloc** _(Write)_: If set to $true then max memory will be dynamically configured. When this is set parameter is set to $true, the parameter MaxMemory and MinMemory must be set to $null or not be configured.

MinMemory does not need to be $null, right? So that should be removed. User is allowed to set the minimum memory.
Add that the default value is $false.

README.md, line 612 at r3 (raw file):

When set to 'Absent' min and max memory will be set to default value.

When set to 'Absent' min and max memory will be set to default values.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 5 at r3 (raw file):

This function gets the min and max memory of server configuration option.

This function gets the value of the min and max memory server configuration option.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 20 at r3 (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Parameter with capital P

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 25 at r3 (raw file):

        $SQLInstanceName,

        [parameter(Mandatory = $true)]

Change to [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 28 at r3 (raw file):

        [ValidateNotNullOrEmpty()]
        [System.String]
        $SQLServer

Add default value of env:COMPUTERNAME
Also add default value to schema.mof and README.md

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 35 at r3 (raw file):

'Getting the min and max of memory server configuration option'

'Getting the value for minimum and maximum SQL server memory.'

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 52 at r3 (raw file):

'This function sets the min and max memory of server configuration option.'

'This function sets the value for the min and max memory server configuration option.'

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 61 at r3 (raw file):

This is The Ensure Set to 'present' to specificy that the min and max memory should be configured.

Please use the same description as in the README.md

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 65 at r3 (raw file):

    .PARAMETER DynamicAlloc
    If set to $true then max memory will be dynamically configured.
    When this is set parameter is set to $true, the parameter MaxMemory and MinMemory must be set to $null or not be configured.

MinMemory does not need to be $null, right? So that should be removed. User is allowed to set the minimum memory.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 78 at r3 (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Parameter with capital P

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 83 at r3 (raw file):

        $SQLInstanceName,

        [parameter(Mandatory = $true)]

Change to [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 86 at r3 (raw file):

        [ValidateNotNullOrEmpty()]
        [System.String]
        $SQLServer,

Add default value of env:COMPUTERNAME

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 88 at r3 (raw file):

        $SQLServer,

        [ValidateSet('Present','Absent')]

Add [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 92 at r3 (raw file):

        $Ensure = 'Present',

        [System.Boolean]

Add [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 95 at r3 (raw file):

        $DynamicAlloc = $false,

        [System.Int32]

Add [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 98 at r3 (raw file):

        $MinMemory,

        [System.Int32]

Add [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 106 at r3 (raw file):

'Setting the min and max of memory server configuration option'

'Setting the minimum and maximum memory used by the instance.'

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 115 at r3 (raw file):

                    if ($MaxMemory)
                    {
                        throw New-TerminatingError -ErrorType 'MaxMemoryParamMustBeNull' `

There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 121 at r3 (raw file):

"Dynamic MaxMemory is $MaxMemory."

"Dynamic maximum memory has been calculated to $($MaxMemory)MB."

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 127 at r3 (raw file):

                    if (-not $MaxMemory)
                    {
                        throw New-TerminatingError -ErrorType 'MaxMemoryParamMustBeNotNull' `

The Key should be 'MaxMemoryParamMustNotBeNull'
There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 138 at r3 (raw file):

'Ensure is absent - Minimum and maximum server memory reset to default value'

'Ensure is set to absent. so minimum and maximum server memory values used by the instance are reset to the default values.'

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 145 at r3 (raw file):

        {
            $sqlServerObject.Configuration.MaxServerMemory.ConfigValue = $MaxMemory
            $sqlServerObject.Configuration.MinServerMemory.ConfigValue = $MinMemory

You should have a check above to see if MinMemory is not set, and then set it to the default that is set in the instance. If a user sets MinMemory thru GUI to, let say, 1024MB, and then run the configuration with out specifying MinMemory, then the resource will reset it to 0, right? Because $null will be translated to 0.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 147 at r3 (raw file):

"SQL Server Memory has been capped to $MaxMemory. Minimum server memory set to $MinMemory."

"Maximum memory used by the instance has been limited to $($MaxMemory)MB. Minimum memory used by the instance is set to $($MinMemory)MB."

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 151 at r3 (raw file):

        catch
        {
            throw New-TerminatingError -ErrorType 'ServerMemorySetError' `

There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 161 at r3 (raw file):

This function tests the min and max memory of server configuration option.

This function tests the value of the min and max memory server configuration option.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 170 at r3 (raw file):

This is The Ensure Set to 'present' to specificy that the min and max memory should be configured.

Use the same description as in the README.md

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 174 at r3 (raw file):

    .PARAMETER DynamicAlloc
    If set to $true then max memory will be dynamically configured.
    When this is set parameter is set to $true, the parameter MaxMemory and MinMemory must be set to $null or not be configured.

MinMemory does not need to be $null, right? So that should be removed. User is allowed to set the minimum memory.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 188 at r3 (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Parameter with capital P

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 193 at r3 (raw file):

        $SQLInstanceName,

        [parameter(Mandatory = $true)]

Change to [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 196 at r3 (raw file):

        [ValidateNotNullOrEmpty()]
        [System.String]
        $SQLServer,

Add default value of env:COMPUTERNAME

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 198 at r3 (raw file):

        $SQLServer,

        [ValidateSet("Present","Absent")]

Add [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 202 at r3 (raw file):

        $Ensure = 'Present',

        [System.Boolean]

Add [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 205 at r3 (raw file):

        $DynamicAlloc = $false,

        [System.Int32]

Add [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 208 at r3 (raw file):

        $MinMemory,

        [System.Int32]

Add [Parameter()]

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 212 at r3 (raw file):

'Testing the min and max of memory server configuration option'··

"Testing the values of the minimum and maximum memory server configuration option set to be used by the instance."

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 214 at r3 (raw file):

$parameters

Please change to $getTargetReosurceParameters

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 219 at r3 (raw file):

$currentValues

Please change to $getTargetResourceResult

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 221 at r3 (raw file):

$getMinMemory

Change to $currentMinMemory

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 222 at r3 (raw file):

    $getMinMemory = $currentValues.MinMemory
    $getMaxMemory = $currentValues.MaxMemory

Change to $currentMaxMemory

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 229 at r3 (raw file):

        'Absent'
        {
            if ($getMaxMemory -ne<PHONE_NUMBER>)

Change to $currentMaxMemory

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 231 at r3 (raw file):

"Current maximum server memory is $getMaxMemory. Expected<PHONE_NUMBER>"

"Current maximum server memory used by the instance is $($currentMaxMemory)MB. Expected<PHONE_NUMBER>MB."

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 235 at r3 (raw file):

            }

            if ($getMinMemory -ne 0)

Change to $currentMinMemory

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 237 at r3 (raw file):

"Current minimum server memory is $getMinMemory. Expected 0"

"Current minimum server memory used by the instance is $($currentMinMemory)MB. Expected 0MB."

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 248 at r3 (raw file):

                if ($MaxMemory)
                {
                    throw New-TerminatingError -ErrorType 'MaxMemoryParamMustBeNull' `

There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 254 at r3 (raw file):

"Dynamic MaxMemory is $MaxMemory."

"Dynamic maximum memory has been calculated to $($MaxMemory)MB."

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 259 at r3 (raw file):

"Current maximum server memory is $getMaxMemory, expected $MaxMemory"

"Current maximum server memory used by the instance is $($currentMaxMemory)MB. Expected $(MaxMemory)MB."

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 263 at r3 (raw file):

            }

            if ($MinMemory -ne $getMinMemory)

This check should not be evaluated if the user has not assigned any value to the parameter MinMemory.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 265 at r3 (raw file):

"Current minimum server memory is $getMinMemory, expected $MinMemory"

"Current minimum server memory used by the instance is $($currentMinMemory)MB. Expected $(MinMemory)MB."

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 283 at r3 (raw file):

    {
        $physicalMemory = ((Get-CimInstance -ClassName Win32_PhysicalMemory).Capacity | Measure-Object -Sum).Sum
        $physicalMemoryMB = [Math]::round($physicalMemory / 1MB)

Please change $physicalMemoryMB to $physicalMemoryInMegaBytes
[Math]::Round with capital 'R'

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 286 at r3 (raw file):

        # Find how much to save for OS: 20% of total ram for under 15GB / 12.5% for over 20GB
        if ($physicalMemoryMB -ge 20480)

Please change $physicalMemoryMB to $physicalMemoryInMegaBytes

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 288 at r3 (raw file):

        if ($physicalMemoryMB -ge 20480)
        {
            $osMemReserved = [Math]::round((0.125 * $physicalMemoryMB))

Please change $osMemoryReserved to $reservedOperatingSystemMemory
Please change $physicalMemoryMB to $physicalMemoryInMegaBytes
[Math]::Round with capital 'R'

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 292 at r3 (raw file):

        else
        {
            $osMemReserved = [Math]::round((0.2 * $physicalMemoryMB))

Please change $physicalMemoryMB to $physicalMemoryInMegaBytes
[Math]::Round with capital 'R'

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 292 at r3 (raw file):

        else
        {
            $osMemReserved = [Math]::round((0.2 * $physicalMemoryMB))

Please change $osMemoryReserved to $reservedOperatingSystemMemory

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 295 at r3 (raw file):

$cimInstanceProc = Get-CimInstance -ClassName Win32_Processor
$numCores = (Measure-Object -InputObject $cimInstanceProc -Property NumberOfCores -Sum).Sum

Chnage these two lines to (?):
$numberOfCores = (Get-CimInstance -ClassName Win32_Processor | Measure-Object -Property NumberOfCores -Sum).Sum

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 298 at r3 (raw file):

        $numCores = (Measure-Object -InputObject $cimInstanceProc -Property NumberOfCores -Sum).Sum

        # Find Number of SQL Threads = 256 + (NumofProcesors - 4) * 8

This comment is not correct

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 299 at r3 (raw file):

        # Find Number of SQL Threads = 256 + (NumofProcesors - 4) * 8
        if ($numCores -ge 4)

Change $numCores to $numberOfCores

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 301 at r3 (raw file):

        if ($numCores -ge 4)
        {
            $numOfSQLThreads = 256 + ($numCores - 4) * 8

Change $numOfSQLThreads to $numberOfSqlThreads
Change $numCores to $numberOfCores

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 305 at r3 (raw file):

        else
        {
            $numOfSQLThreads = 0

Change $numOfSQLThreads to $numberOfSqlThreads

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 308 at r3 (raw file):

$osArchitecture

Change to $operatingSystemArchitecture

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 311 at r3 (raw file):

        # Find ThreadStackSize 1MB x86/ 2MB x64/ 4MB IA64
        if ($osArchitecture -eq '32-bit')

Change to $operatingSystemArchitecture

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 313 at r3 (raw file):

$ThreadStackSize

Change to $threadStackSize

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 315 at r3 (raw file):

            $ThreadStackSize = 1
        }
        elseif ($osArchitecture -eq '64-bit')

Change to $operatingSystemArchitecture

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 317 at r3 (raw file):

$ThreadStackSize

Change to $threadStackSize

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 321 at r3 (raw file):

$ThreadStackSize

Change to $threadStackSize

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 324 at r3 (raw file):

$maxMemory = $physicalMemoryMB - $osMemReserved - ($numOfSQLThreads * $ThreadStackSize) - (1024 * [System.Math]::Ceiling($numCores / 4))

$maxMemory = $physicalMemoryInMegabytes - $reservedOperatingSystemMemory - ($numberOfSqlThreads * $threadStackSize) - (1024 * [System.Math]::Ceiling($numberOfCores / 4))

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 328 at r3 (raw file):

    catch
    {
        throw New-TerminatingError -ErrorType 'ErrorGetDynamicMaxMemory' `

There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.schema.mof, line 5 at r3 (raw file):

{
    [Key, Description("The name of the SQL instance to be configured.")] String SQLInstanceName;
    [Required, Description("The host name of the SQL Server to be configured.")] String SQLServer;

Please make SQLServer parameter to the type qualifier Write.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.schema.mof, line 6 at r3 (raw file):

    [Key, Description("The name of the SQL instance to be configured.")] String SQLInstanceName;
    [Required, Description("The host name of the SQL Server to be configured.")] String SQLServer;
    [Write, Description("An enumerated value that describes if Min and Max memory is configured"), ValueMap{"Present","Absent"}, Values{"Present","Absent"}] String Ensure;

Change this description to match the readme.md

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.schema.mof, line 7 at r3 (raw file):

    [Required, Description("The host name of the SQL Server to be configured.")] String SQLServer;
    [Write, Description("An enumerated value that describes if Min and Max memory is configured"), ValueMap{"Present","Absent"}, Values{"Present","Absent"}] String Ensure;
    [Write, Description("Flag to Dynamically allocate SQL memory based on Best Practices")] Boolean DynamicAlloc;

Change this description to match the readme.md
Add that the default value is $false.

Examples/Resources/xSQLServerMemory/2-SetMaxMemoryToAuto.ps1, line 23 at r3 (raw file):

        {
            Ensure = 'Present'
            DynamicAlloc = $true

Please add an example which sets both MaxMemory using dynamic and also sets MinMemory to a value other than the default (0).

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 29 at r3 (raw file):

    # Loading mocked classes
    Add-Type -Path (Join-Path -Path $script:moduleRoot -ChildPath 'Tests\Unit\Stubs\SMO.cs')

Change to:
Add-Type -Path (Join-Path -Path (Join-Path -Path (Join-Path -Path (Join-Path -Path $script:moduleRoot -ChildPath 'Tests') -ChildPath 'Unit') -ChildPath 'Stubs') -ChildPath 'SMO.cs')

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 43 at r3 (raw file):

[pscustomobject]

[PSCustomObject]

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 65 at r3 (raw file):

        } -ModuleName $script:DSCResourceName -Verifiable

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

You can remove this entire Context-block. The test does not test anything that the next test does. See next comment.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 88 at r3 (raw file):

        }

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

Change the description to 'When the system is either in the desired state or not in the desired state'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 93 at r3 (raw file):

'Should return the desired MinMemory as 2048'

'Should return the correct value for MinMemory'.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 97 at r3 (raw file):

'Should return the desired MaxMemory as 10300'

'Should return the correct value for MaxMemory'.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 116 at r3 (raw file):

    Describe "$($script:DSCResourceName)\Test-TargetResource" {
        Mock -CommandName Connect-SQL -MockWith {
            $mockSqlServerObject = [pscustomobject]@{

[PSCustomObject]

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 261 at r3 (raw file):

        Mock -CommandName Connect-SQL -MockWith {
            $mockSqlServerObject = [pscustomobject]@{

[PSCustomObject]

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 305 at r3 (raw file):

        Mock -CommandName Connect-SQL -MockWith {
            $mockSqlServerObject = [pscustomobject]@{

[PSCustomObject]

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 348 at r3 (raw file):

        Mock -CommandName Connect-SQL -MockWith {
            $mockSqlServerObject = [pscustomobject]@{

[PSCustomObject]

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 398 at r3 (raw file):

    Describe "$($script:DSCResourceName)\Set-TargetResource" {
        Mock -CommandName Connect-SQL -MockWith {
            $mockSqlServerObject = [pscustomobject]@{

[PSCustomObject]

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 560 at r3 (raw file):

        Mock -CommandName Connect-SQL -MockWith {
            $mockSqlServerObject = [pscustomobject]@{

[PSCustomObject]

Comments from Reviewable

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

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 115 at r3 (raw file):

    Describe "$($script:DSCResourceName)\Test-TargetResource" {
        Mock -CommandName Connect-SQL -MockWith {

Add these for both Test- and Set-methods.
Please also add tests where the mock sets MinServerMemory to, for example, 2048.
Do a test where the parameter MinMemory is not specified in the parameters (this will show the bug in the code I commented about)
If you do this before fixing the code comments, you should see that the test fails. And then fix the code, and see that is passes. Test-Driven Development. :smile:  :wink:

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 218 at r3 (raw file):

'When the MaxMemory paramater is not null and DynamicAlloc set to true'

'When the MaxMemory parameter is not null and DynamicAlloc is set to true'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 226 at r3 (raw file):

'Should Throw when MaxMemory paramater not null if DynamicAlloc set to true'

'Should throw when MaxMemory parameter is not null and DynamicAlloc is set to true'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 227 at r3 (raw file):

            It 'Should Throw when MaxMemory paramater not null if DynamicAlloc set to true' {
                { Test-TargetResource @testParameters } | Should Throw

Make sure you check so that it throws the correct error here.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 256 at r3 (raw file):

            It 'Should call the mock function Get-CimInstance' {
                Assert-MockCalled Get-CimInstance -Exactly -Times 3 -ModuleName $script:DSCResourceName -Scope Context

Please add -ParameterFilter to this Assert and split it up in three Asserts so you know that Get-CimInstance was called with the correct parameters for each call.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 300 at r3 (raw file):

            It 'Should call the mock function Get-CimInstance' {
                Assert-MockCalled Get-CimInstance -Exactly -Times 3 -ModuleName $script:DSCResourceName -Scope Context

Please add -ParameterFilter to this Assert and split it up in three Asserts so you know that Get-CimInstance was called with the correct parameters for each call.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 333 at r3 (raw file):

'Should return the state as false when desired MinMemory and MaxMemory are not present'

'Should return the state as false when desired MinMemory and MaxMemory are not set to the default values'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 457 at r3 (raw file):

'When the MaxMemory paramater is not null and DynamicAlloc set to true'

'When the MaxMemory parameter is not null and DynamicAlloc is set to true'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 465 at r3 (raw file):

'Should Throw when MaxMemory paramater not null if DynamicAlloc set to true'

'Should throw when MaxMemory parameter is not null and DynamicAlloc is set to true'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 466 at r3 (raw file):

            It 'Should Throw when MaxMemory paramater not null if DynamicAlloc set to true' {
                { Set-TargetResource @testParameters } | Should Throw

Make sure you check so that it throws the correct error here.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 478 at r3 (raw file):

'When the MaxMemory paramater is null and DynamicAlloc set to false'

'When the MaxMemory parameter is null and DynamicAlloc is set to false'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 485 at r3 (raw file):

            }

            It 'Should Throw when MaxMemory paramater not null if DynamicAlloc set to true' {

This description doesn't seem to correlate with the Context description. Please revise.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 486 at r3 (raw file):

            It 'Should Throw when MaxMemory paramater not null if DynamicAlloc set to true' {
                { Set-TargetResource @testParameters } | Should Throw

Make sure you check so that it throws the correct error here.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 498 at r3 (raw file):

'When the Ensure parameter is set to Absent'

'When the system is not in the desired state and Ensure is set to Absent'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 504 at r3 (raw file):

'Should Not Throw when Ensure parameter is set to Absent'

'Should set the MinMemory and MaxMemory to the default values'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 505 at r3 (raw file):

            It 'Should Not Throw when Ensure parameter is set to Absent' {
                { Set-TargetResource @testParameters } | Should Not Throw

You have to test so that what you alter is what you wanted to alter.  You have to add a dynamic variable so you can verify that the value MinServerMemory and MaxServerMemory is the correct expected values.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 517 at r3 (raw file):

'When the desired MinMemory and MaxMemory parameter are not set'

'When the system is not in the desired state and Ensure is set to Present, and DynamicAlloc is set to false'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 526 at r3 (raw file):

'Should Not Throw when MaxMemory paramater is not null and DynamicAlloc set to false'

'Should set the MinMemory and MaxMemory to the correct values when Ensure parameter is set to Present and DynamicAlloc is set to false.'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 527 at r3 (raw file):

            It 'Should Not Throw when MaxMemory paramater is not null and DynamicAlloc set to false' {
                { Set-TargetResource @testParameters } | Should Not Throw

You have to test so that what you alter is what you wanted to alter.  You have to add a dynamic variable so you can verify that the value MinServerMemory and MaxServerMemory is the correct expected values.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 539 at r3 (raw file):

'When the system is not in the desired state and DynamicAlloc is set to true'

'When the system is not in the desired state and Ensure is set to Present, and DynamicAlloc is set to true'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 547 at r3 (raw file):

            It 'Should Not Throw when MaxMemory paramater is not null and DynamicAlloc set to false' {
                { Set-TargetResource @testParameters } | Should Not Throw

You have to test so that what you alter is what you wanted to alter.  You have to add a dynamic variable so you can verify that the value MinServerMemory and MaxServerMemory is the correct expected values.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 587 at r3 (raw file):

'When the desired MinMemory and MaxMemory parameter are not set'

'When the desired MinMemory and MaxMemory fails to be set'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 596 at r3 (raw file):

'Should Throw when Alter Method was called with invalid operation'

'Should throw when Alter method was called with an invalid operation'

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 597 at r3 (raw file):

            It 'Should Throw when Alter Method was called with invalid operation' {
                { Set-TargetResource @testParameters } | Should Throw

Make sure you check so that yu get the correct error here.

Comments from Reviewable

Review status: 6 of 8 files reviewed at latest revision, 117 unresolved discussions.

README.md, line 597 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

(NumOfSQLThreads x ThreadStackSize)

Change to:
(NumOfSQLThreads * ThreadStackSize)

No it's not possible to use *, appveyor failed because of markdown test

Comments from Reviewable

Review status: 6 of 8 files reviewed at latest revision, 117 unresolved discussions.

README.md, line 595 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This way of dynamically setting the values, is that a best practice. Is there an article you can link to about it? If not, can you add a text to say how you come by to using this particular formula?

I find the formula here
http://sqlmax.chuvash.eu/

Comments from Reviewable

Review status: 6 of 8 files reviewed at latest revision, 117 unresolved discussions.

README.md, line 597 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Could you write the formula in more clear text, like you did for the MaxDop PR? As of now it is kind of hard to read it.

Maybe I can remove the formula from readme, is it really necessary for end-users?

Comments from Reviewable

Review status: 6 of 8 files reviewed at latest revision, 121 unresolved discussions, some commit checks failed.

README.md, line 598 at r5 (raw file):

#### Formula for dynamically allocating maximum memory

* The dynamic maximum memory (in MB) is calculate with this formula:

Tests complain about trailing spaces at the end of the row.
598: MD009 Trailing spaces

README.md, line 599 at r5 (raw file):

* The dynamic maximum memory (in MB) is calculate with this formula:
SQL Max Memory = TotalPhysicalMemory - (NumOfSQLThreads x ThreadStackSize) - (1024 x CEILING(NumOfCores/4)) - OSReservedMemory.

You get these error becuase this line is on a seperate row.
MD032 Lists should be surrounded by blank lines
MD032 Lists should be surrounded by blank lines
To solve this, either move this line to the end of the previous line.
Or, remove the * from the row above (so it is not a list item), and then make a blank row after this row.

README.md, line 599 at r5 (raw file):

* The dynamic maximum memory (in MB) is calculate with this formula:
SQL Max Memory = TotalPhysicalMemory - (NumOfSQLThreads x ThreadStackSize) - (1024 x CEILING(NumOfCores/4)) - OSReservedMemory.

Does it work using * if you escape it using backslash *?

README.md, line 609 at r5 (raw file):

* **OSReservedMemory**
* If the total physical memory is under 20 GB, the percentage of reserved memory for OS is 20% of total physical memory.
* If the total physical memory is upper 20 GB, the percentage of reserved memory for OS is 12.5% of total physical memory.

Same here, trailing space at the end of the row
609: MD009 Trailing spaces

Comments from Reviewable

Review status: 6 of 8 files reviewed at latest revision, 121 unresolved discussions, some commit checks failed.

README.md, line 597 at r3 (raw file):
Previously, luigilink (Jean-Cyril Drouhin) wrote…
No it's not possible to use *, appveyor failed because of markdown test

Does it work using * if you escape it using backslash \*?

README.md, line 597 at r3 (raw file):
Previously, luigilink (Jean-Cyril Drouhin) wrote…
Maybe I can remove the formula from readme, is it really necessary for end-users?

I think having the formula in the README is beneficial for the user. If they don't know how it is being calculated, fewer would want to use it I think, and even fewer could help make it better.
So please keep it in. :)

Comments from Reviewable

Review status: 6 of 8 files reviewed at latest revision, 122 unresolved discussions, some commit checks failed.

README.md, line 601 at r5 (raw file):

SQL Max Memory = TotalPhysicalMemory - (NumOfSQLThreads x ThreadStackSize) - (1024 x CEILING(NumOfCores/4)) - OSReservedMemory.
* **NumOfSQLThreads**
* If the number of cores is less than and equal to 4, the number of Sql threads is the number of cores minus 4 multiply by 8, plus 256.

According to the link you provided in a separate comment, the formula should use "number of processors", not "number of cores". Wrong in code as well?

Comments from Reviewable

Review status: 6 of 8 files reviewed at latest revision, 122 unresolved discussions.

README.md, line 601 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
According to the link you provided in a separate comment, the formula should use "number of processors", not "number of cores". Wrong in code as well?

I check with the source code of the project numOfProc = numOfCores oO. I can calculate the number of processors with ciminsntance but the result will not be the same.
And if you look and the form, there is not "Number of Processors" input label.

Comments from Reviewable

Review status: 6 of 8 files reviewed at latest revision, 122 unresolved discussions.

CHANGELOG.md, line 3 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please rebase and make sure your changes stays under the Unreleased section, and does not get moved to the <IP_ADDRESS> version.

Done.

CHANGELOG.md, line 74 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This can be removed

Done.

CHANGELOG.md, line 116 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please remove SQLServer frpn the mandatory parameters.
Please add that DynamicAlloc is no longer mandatory.

Done.

Comments from Reviewable

Review status: 6 of 8 files reviewed at latest revision, 122 unresolved discussions, some commit checks failed.

README.md, line 591 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

This resource set the min server memory and max server memory configuration option.

This resource sets the min server memory and max server memory configuration option. That means it sets the minimum and the maximum amount of memory, in MB, in the buffer pool used by the instance of SQL Server.

Done.

README.md, line 593 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change 'min' to 'minimum'.  Also a two space before, change to one space.
Change 'max' to 'maximum'

Done.

README.md, line 610 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to type qualifier Write.

Done.

README.md, line 611 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
MinMemory does not need to be $null, right? So that should be removed. User is allowed to set the minimum memory.
Add that the default value is $false.

Done.

README.md, line 612 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

When set to 'Absent' min and max memory will be set to default value.

When set to 'Absent' min and max memory will be set to default values.

Done.

README.md, line 598 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Tests complain about trailing spaces at the end of the row.
598: MD009 Trailing spaces

Done.

README.md, line 599 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
You get these error becuase this line is on a seperate row.
MD032 Lists should be surrounded by blank lines
MD032 Lists should be surrounded by blank lines
To solve this, either move this line to the end of the previous line.
Or, remove the * from the row above (so it is not a list item), and then make a blank row after this row.

Done.

README.md, line 609 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Same here, trailing space at the end of the row
609: MD009 Trailing spaces

Done.

Comments from Reviewable

@johlju Do you have a technic to test markdown locally ? I installed node.js but it can't found npm any idea ?
 Markdownlint is also used in the test. So that should make you be able to get these lint errors. :)
In VSCode it will show up as error in the "error view" (don't remember what the view is called) as well they will show up directly as squiggly lines in the markdown file.
I do have the ability to run the test locally also, but don't remember what I installed to make it work, node.js sound familiar, though. Can check on that.

Read more about markdownlint here:
https://github.com/PowerShell/xSQLServer/blob/dev/CONTRIBUTING.md#documentation-with-markdown
And you should just install node.js and mom should be available to run the test. See the same link above for link to node.js.

Reviewed 1 of 3 files at r6, 2 of 3 files at r7, 2 of 3 files at r8.
Review status: all files reviewed at latest revision, 122 unresolved discussions.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 43 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

[pscustomobject]

[PSCustomObject]

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 65 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
You can remove this entire Context-block. The test does not test anything that the next test does. See next comment.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 88 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change the description to 'When the system is either in the desired state or not in the desired state'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 93 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Should return the desired MinMemory as 2048'

'Should return the correct value for MinMemory'.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 97 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Should return the desired MaxMemory as 10300'

'Should return the correct value for MaxMemory'.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 115 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add these for both Test- and Set-methods.
Please also add tests where the mock sets MinServerMemory to, for example, 2048.
Do a test where the parameter MinMemory is not specified in the parameters (this will show the bug in the code I commented about)
If you do this before fixing the code comments, you should see that the test fails. And then fix the code, and see that is passes. Test-Driven Development. :smile:  :wink:

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 116 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
[PSCustomObject]

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 218 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'When the MaxMemory paramater is not null and DynamicAlloc set to true'

'When the MaxMemory parameter is not null and DynamicAlloc is set to true'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 226 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Should Throw when MaxMemory paramater not null if DynamicAlloc set to true'

'Should throw when MaxMemory parameter is not null and DynamicAlloc is set to true'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 261 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
[PSCustomObject]

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 305 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
[PSCustomObject]

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 333 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Should return the state as false when desired MinMemory and MaxMemory are not present'

'Should return the state as false when desired MinMemory and MaxMemory are not set to the default values'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 348 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
[PSCustomObject]

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 398 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
[PSCustomObject]

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 457 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'When the MaxMemory paramater is not null and DynamicAlloc set to true'

'When the MaxMemory parameter is not null and DynamicAlloc is set to true'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 465 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Should Throw when MaxMemory paramater not null if DynamicAlloc set to true'

'Should throw when MaxMemory parameter is not null and DynamicAlloc is set to true'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 478 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'When the MaxMemory paramater is null and DynamicAlloc set to false'

'When the MaxMemory parameter is null and DynamicAlloc is set to false'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 498 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'When the Ensure parameter is set to Absent'

'When the system is not in the desired state and Ensure is set to Absent'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 504 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Should Not Throw when Ensure parameter is set to Absent'

'Should set the MinMemory and MaxMemory to the default values'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 517 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'When the desired MinMemory and MaxMemory parameter are not set'

'When the system is not in the desired state and Ensure is set to Present, and DynamicAlloc is set to false'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 526 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Should Not Throw when MaxMemory paramater is not null and DynamicAlloc set to false'

'Should set the MinMemory and MaxMemory to the correct values when Ensure parameter is set to Present and DynamicAlloc is set to false.'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 539 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'When the system is not in the desired state and DynamicAlloc is set to true'

'When the system is not in the desired state and Ensure is set to Present, and DynamicAlloc is set to true'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 560 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
[PSCustomObject]

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 587 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'When the desired MinMemory and MaxMemory parameter are not set'

'When the desired MinMemory and MaxMemory fails to be set'

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 596 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Should Throw when Alter Method was called with invalid operation'

'Should throw when Alter method was called with an invalid operation'

Done.

Comments from Reviewable

Review status: all files reviewed at latest revision, 122 unresolved discussions.

README.md, line 592 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change 'min' to 'minimum'
Change 'max' to 'maximum'

Done.

README.md, line 597 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Does it work using * if you escape it using backslash \*?

Done.

README.md, line 597 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
I think having the formula in the README is beneficial for the user. If they don't know how it is being calculated, fewer would want to use it I think, and even fewer could help make it better.
So please keep it in. :)

Done.

README.md, line 599 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Does it work using * if you escape it using backslash *?

Done.

README.md, line 601 at r5 (raw file):
Previously, luigilink (Jean-Cyril Drouhin) wrote…
I check with the source code of the project numOfProc = numOfCores oO. I can calculate the number of processors with ciminsntance but the result will not be the same.
And if you look and the form, there is not "Number of Processors" input label.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 5 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

This function gets the min and max memory of server configuration option.

This function gets the value of the min and max memory server configuration option.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 25 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 28 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add default value of env:COMPUTERNAME
Also add default value to schema.mof and README.md

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 35 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Getting the min and max of memory server configuration option'

'Getting the value for minimum and maximum SQL server memory.'

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 52 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'This function sets the min and max memory of server configuration option.'

'This function sets the value for the min and max memory server configuration option.'

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 61 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

This is The Ensure Set to 'present' to specificy that the min and max memory should be configured.

Please use the same description as in the README.md

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 65 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
MinMemory does not need to be $null, right? So that should be removed. User is allowed to set the minimum memory.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 78 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Parameter with capital P

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 86 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add default value of env:COMPUTERNAME

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 88 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 92 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 95 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 98 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 106 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Setting the min and max of memory server configuration option'

'Setting the minimum and maximum memory used by the instance.'

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 138 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Ensure is absent - Minimum and maximum server memory reset to default value'

'Ensure is set to absent. so minimum and maximum server memory values used by the instance are reset to the default values.'

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 161 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

This function tests the min and max memory of server configuration option.

This function tests the value of the min and max memory server configuration option.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 170 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

This is The Ensure Set to 'present' to specificy that the min and max memory should be configured.

Use the same description as in the README.md

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 174 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
MinMemory does not need to be $null, right? So that should be removed. User is allowed to set the minimum memory.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 188 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Parameter with capital P

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 193 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 196 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add default value of env:COMPUTERNAME

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 198 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 202 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 205 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 208 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 214 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$parameters

Please change to $getTargetReosurceParameters

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 219 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$currentValues

Please change to $getTargetResourceResult

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 222 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to $currentMaxMemory

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 229 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to $currentMaxMemory

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 231 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

"Current maximum server memory is $getMaxMemory. Expected<PHONE_NUMBER>"

"Current maximum server memory used by the instance is $($currentMaxMemory)MB. Expected<PHONE_NUMBER>MB."

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 235 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to $currentMinMemory

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 237 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

"Current minimum server memory is $getMinMemory. Expected 0"

"Current minimum server memory used by the instance is $($currentMinMemory)MB. Expected 0MB."

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 254 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

"Dynamic MaxMemory is $MaxMemory."

"Dynamic maximum memory has been calculated to $($MaxMemory)MB."

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 259 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

"Current maximum server memory is $getMaxMemory, expected $MaxMemory"

"Current maximum server memory used by the instance is $($currentMaxMemory)MB. Expected $(MaxMemory)MB."

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 263 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This check should not be evaluated if the user has not assigned any value to the parameter MinMemory.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 265 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

"Current minimum server memory is $getMinMemory, expected $MinMemory"

"Current minimum server memory used by the instance is $($currentMinMemory)MB. Expected $(MinMemory)MB."

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 283 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please change $physicalMemoryMB to $physicalMemoryInMegaBytes
[Math]::Round with capital 'R'

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 288 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please change $osMemoryReserved to $reservedOperatingSystemMemory
Please change $physicalMemoryMB to $physicalMemoryInMegaBytes
[Math]::Round with capital 'R'

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 292 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please change $physicalMemoryMB to $physicalMemoryInMegaBytes
[Math]::Round with capital 'R'

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 292 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please change $osMemoryReserved to $reservedOperatingSystemMemory

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 295 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$cimInstanceProc = Get-CimInstance -ClassName Win32_Processor
$numCores = (Measure-Object -InputObject $cimInstanceProc -Property NumberOfCores -Sum).Sum

Chnage these two lines to (?):
$numberOfCores = (Get-CimInstance -ClassName Win32_Processor | Measure-Object -Property NumberOfCores -Sum).Sum

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 298 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This comment is not correct

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 299 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change $numCores to $numberOfCores

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 308 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$osArchitecture

Change to $operatingSystemArchitecture

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 311 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to $operatingSystemArchitecture

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 313 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$ThreadStackSize

Change to $threadStackSize

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 315 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to $operatingSystemArchitecture

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 317 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$ThreadStackSize

Change to $threadStackSize

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 321 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$ThreadStackSize

Change to $threadStackSize

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.schema.mof, line 5 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please make SQLServer parameter to the type qualifier Write.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.schema.mof, line 6 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change this description to match the readme.md

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.schema.mof, line 7 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change this description to match the readme.md
Add that the default value is $false.

Done.

Comments from Reviewable

Review status: all files reviewed at latest revision, 122 unresolved discussions.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 29 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to:
Add-Type -Path (Join-Path -Path (Join-Path -Path (Join-Path -Path (Join-Path -Path $script:moduleRoot -ChildPath 'Tests') -ChildPath 'Unit') -ChildPath 'Stubs') -ChildPath 'SMO.cs')

Done.

Comments from Reviewable

Review status: all files reviewed at latest revision, 122 unresolved discussions.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 286 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please change $physicalMemoryMB to $physicalMemoryInMegaBytes

Done.

Comments from Reviewable

Codecov Report

Merging #343 into dev will increase coverage by 2%.
The diff coverage is 100%.

@@         Coverage Diff         @@
##            dev   #343   +/-   ##
===================================
+ Coverage    63%    66%   +2%
===================================
  Files        31     31
  Lines      3092   3113   +21
===================================
+ Hits       1978   2061   +83
+ Misses     1114   1052   -62

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4304e00...ad1fc22. Read the comment docs.

Reviewed 3 of 7 files at r2, 4 of 4 files at r9.
Review status: all files reviewed at latest revision, 122 unresolved discussions.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 20 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Parameter with capital P

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 83 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to [Parameter()]

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 115 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 127 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
The Key should be 'MaxMemoryParamMustNotBeNull'
There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 151 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 221 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$getMinMemory

Change to $currentMinMemory

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 248 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 301 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change $numOfSQLThreads to $numberOfSqlThreads
Change $numCores to $numberOfCores

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 305 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change $numOfSQLThreads to $numberOfSqlThreads

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 324 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$maxMemory = $physicalMemoryMB - $osMemReserved - ($numOfSQLThreads * $ThreadStackSize) - (1024 * [System.Math]::Ceiling($numCores / 4))

$maxMemory = $physicalMemoryInMegabytes - $reservedOperatingSystemMemory - ($numberOfSqlThreads * $threadStackSize) - (1024 * [System.Math]::Ceiling($numberOfCores / 4))

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 328 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
There is no localization string for this error in the file en-US/xSQLServer.strings.psd1 (?)

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 227 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Make sure you check so that it throws the correct error here.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 256 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please add -ParameterFilter to this Assert and split it up in three Asserts so you know that Get-CimInstance was called with the correct parameters for each call.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 300 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please add -ParameterFilter to this Assert and split it up in three Asserts so you know that Get-CimInstance was called with the correct parameters for each call.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 466 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Make sure you check so that it throws the correct error here.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 485 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This description doesn't seem to correlate with the Context description. Please revise.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 486 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Make sure you check so that it throws the correct error here.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 597 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Make sure you check so that yu get the correct error here.

Done.

Comments from Reviewable

Reviewed 6 of 6 files at r10.
Review status: all files reviewed at latest revision, 122 unresolved discussions.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 121 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

"Dynamic MaxMemory is $MaxMemory."

"Dynamic maximum memory has been calculated to $($MaxMemory)MB."

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 145 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
You should have a check above to see if MinMemory is not set, and then set it to the default that is set in the instance. If a user sets MinMemory thru GUI to, let say, 1024MB, and then run the configuration with out specifying MinMemory, then the resource will reset it to 0, right? Because $null will be translated to 0.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 147 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

"SQL Server Memory has been capped to $MaxMemory. Minimum server memory set to $MinMemory."

"Maximum memory used by the instance has been limited to $($MaxMemory)MB. Minimum memory used by the instance is set to $($MinMemory)MB."

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 212 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'Testing the min and max of memory server configuration option'··

"Testing the values of the minimum and maximum memory server configuration option set to be used by the instance."

Done.

Examples/Resources/xSQLServerMemory/2-SetMaxMemoryToAuto.ps1, line 23 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please add an example which sets both MaxMemory using dynamic and also sets MinMemory to a value other than the default (0).

Done.

Comments from Reviewable

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

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 505 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
You have to test so that what you alter is what you wanted to alter.  You have to add a dynamic variable so you can verify that the value MinServerMemory and MaxServerMemory is the correct expected values.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 527 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
You have to test so that what you alter is what you wanted to alter.  You have to add a dynamic variable so you can verify that the value MinServerMemory and MaxServerMemory is the correct expected values.

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 547 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
You have to test so that what you alter is what you wanted to alter.  You have to add a dynamic variable so you can verify that the value MinServerMemory and MaxServerMemory is the correct expected values.

Done.

Comments from Reviewable

@johlju ok for review. I hope that I don't miss anything

@luigilink I really appreciate all the work you put into this repo!
Don't worry if you miss anything, nobody can be perfect :) In this case you did awesome. Really dig that you got the concept of dynamic parameters and got it working so quickly! I had a lot of trouble with that in the beginning. :)
Just a bunch of small comments left in this one.

Reviewed 2 of 3 files at r7, 1 of 4 files at r9, 4 of 6 files at r10, 1 of 2 files at r11.
Review status: all files reviewed at latest revision, 14 unresolved discussions.

README.md, line 595 at r3 (raw file):
Could you please add this? (used > before the text to make quoted text)

The formula is based on the SQL Max Memory Calculator website. The website code is in the sql-max GitHub repository maintained by @mirontoli.

Note: In the formula there is mentions of 'Processors', but the code example does not account for physical processors. So this formula here is using Cores just as the JavaScript example here https://github.com/mirontoli/sql-max#formula

README.md, line 601 at r5 (raw file):
Previously, luigilink (Jean-Cyril Drouhin) wrote…
Done.

I agree, if you look at the JavaScript here, there is only Cores mentioned in the code, no reference to physical Processors. In a previous comment I added that as a comment. Just for future reference, so we don't start to discuss the same thing all over again.
Leave this as-is.

README.md, line 637 at r11 (raw file):

If the number of cores is less than and equal to 4, the number of Sql threads is the number of cores minus 4 multiply by 8, plus 256.

I agree with you that it is a little hard to read the correct formula in text. Could we change it to this?
If the number of cores is less than or equal to 4, the number of SQL threads is set to: 256 + (NumberOfCores - 4) * 8.

README.md, line 638 at r11 (raw file):

* **NumOfSQLThreads**
  * If the number of cores is less than and equal to 4, the number of Sql threads is the number of cores minus 4 multiply by 8, plus 256.
  * If the number of cores is greater than 4, the number of Sql threads is equal to 0.

If the number of cores is greater than 4, the number of SQL threads is set to: 0 (zero).

README.md, line 644 at r11 (raw file):

'...under 20 GB...'

'...less than or equal to 20GB...'

README.md, line 645 at r11 (raw file):

'...upper 20 GB...'

'...greater than 20GB...'

README.md, line 661 at r11 (raw file):

* **[Sint32] MaxMemory** _(Write)_: Maximum amount of memory, in MB, in the buffer pool used by the instance of SQL Server.

#### Examples

Please update the examples

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 138 at r3 (raw file):
Previously, luigilink (Jean-Cyril Drouhin) wrote…
Done.

My bad, I wrote a typo. :/ Please remove 'so' in the text. Change to.
'Ensure is set to absent. Minimum and maximum server memory'

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 231 at r3 (raw file):
Previously, luigilink (Jean-Cyril Drouhin) wrote…
Done.

Please add the MB to each value.
"Current maximum server memory used by the instance is $($currentMaxMemory)MB. Expected<PHONE_NUMBER>MB."

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 298 at r3 (raw file):
Previously, luigilink (Jean-Cyril Drouhin) wrote…
Done.

Please change the comment to the following. There is no need to have the formula in the comment since the block also can return 0.
# Get the number of SQL threads.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 137 at r11 (raw file):

                    }
                }
                $sqlServerObject.Configuration.MaxServerMemory.ConfigValue = $MaxMemory

Add a blank row before this one.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 157 at r11 (raw file):

                New-VerboseMessage -Message "Minimum memory used by the instance is set to $($MinMemory)MB."
            }
            $sqlServerObject.Alter()

Please add a blank line before this one.

Examples/Resources/xSQLServerMemory/3-SetMinMaxMemoryToAuto.ps1, line 2 at r11 (raw file):

<#
.EXAMPLE

Filename for this one can be misleading. Maybe change to something like
3-SetMinMemoryToFixedValueAndMaxMemoryToAuto.ps1

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 125 at r11 (raw file):

        Describe "MSFT_xSQLServerMemory\Test-TargetResource" -Tag 'Test'{
            Mock -CommandName Connect-SQL -MockWith $mockConnectSQL -Verifiable

Add a mock just to verify that you don't call the mocked cmdlet with another set of parameter filters
Mock -CommandName Get-CimInstance

If you want to be absolutely sure it is never called in a way you don't want to (if you miss an Assert for example), you could do this.
Mock -CommandName Get-CimInstance -MockWith {
    throw 'Mocked function Get-CimInstance was called with the wrong set of parameter filters.'
}
``

Comments from Reviewable

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

README.md, line 595 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Could you please add this? (used > before the text to make quoted text)

The formula is based on the SQL Max Memory Calculator website. The website code is in the sql-max GitHub repository maintained by @mirontoli.

Note: In the formula there is mentions of 'Processors', but the code example does not account for physical processors. So this formula here is using Cores just as the JavaScript example here https://github.com/mirontoli/sql-max#formula

Done.

README.md, line 601 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
I agree, if you look at the JavaScript here, there is only Cores mentioned in the code, no reference to physical Processors. In a previous comment I added that as a comment. Just for future reference, so we don't start to discuss the same thing all over again.
Leave this as-is.

OK

README.md, line 638 at r11 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
If the number of cores is greater than 4, the number of SQL threads is set to: 0 (zero).

Done.

README.md, line 644 at r11 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'...under 20 GB...'

'...less than or equal to 20GB...'

Done.

README.md, line 645 at r11 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

'...upper 20 GB...'

'...greater than 20GB...'

Done.

README.md, line 661 at r11 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please update the examples

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 138 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
My bad, I wrote a typo. :/ Please remove 'so' in the text. Change to.
'Ensure is set to absent. Minimum and maximum server memory'

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 231 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please add the MB to each value.
"Current maximum server memory used by the instance is $($currentMaxMemory)MB. Expected<PHONE_NUMBER>MB."

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 298 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please change the comment to the following. There is no need to have the formula in the comment since the block also can return 0.
# Get the number of SQL threads.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 137 at r11 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add a blank row before this one.

Done.

DSCResources/MSFT_xSQLServerMemory/MSFT_xSQLServerMemory.psm1, line 157 at r11 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please add a blank line before this one.

Done.

Examples/Resources/xSQLServerMemory/3-SetMinMemoryToFixedValueAndMaxMemoryToAuto.ps1, line 2 at r11 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Filename for this one can be misleading. Maybe change to something like
3-SetMinMemoryToFixedValueAndMaxMemoryToAuto.ps1

Done.

Tests/Unit/MSFT_xSQLServerMemory.Tests.ps1, line 125 at r11 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add a mock just to verify that you don't call the mocked cmdlet with another set of parameter filters
Mock -CommandName Get-CimInstance

If you want to be absolutely sure it is never called in a way you don't want to (if you miss an Assert for example), you could do this.
Mock -CommandName Get-CimInstance -MockWith {
    throw 'Mocked function Get-CimInstance was called with the wrong set of parameter filters.'
}
``

</blockquote></details>

Done.

---

*Comments from [Reviewable](https://reviewable.io:443/reviews/powershell/xsqlserver/343)*
<!-- Sent from Reviewable.io -->

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

README.md, line 637 at r11 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

If the number of cores is less than and equal to 4, the number of Sql threads is the number of cores minus 4 multiply by 8, plus 256.

I agree with you that it is a little hard to read the correct formula in text. Could we change it to this?
If the number of cores is less than or equal to 4, the number of SQL threads is set to: 256 + (NumberOfCores - 4) * 8.

Done.

Comments from Reviewable

@johlju Waiting for the author can be removed 😸

Reviewed 4 of 5 files at r12, 1 of 1 files at r13, 3 of 3 files at r14.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@luigilink Can you fix the CHANGELOG.md? It doesn't think it is a merge conflictàfter I merged your previous PR so I can't fix it in my end. If I merge as-is the CHANGELOG.md will end up wrong.
Thanks.

@johlju Conflict merged resolved 😃

I still got the examples under the following.
Added tests for resources
   xSQLServerMaxDop

Please revise the CHANGELOG.md (git resolved the merge conflicts wrong)

@johlju Sorry I missed that one. Now Changelog.md is up to date.

No worries :)

Reviewed 1 of 1 files at r15.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable


Adding Custom Stages gives me an error and just loads a black background

Describe your problem here.
I'm using Psych Engine 6.3 and I was trying to add a custom stage using the example but it just gives me an error and a black background when trying to load my custom stage
The Code if needed:
function onCreate()
makeLuaSprite('gm_heartlandback', 'gm_heartlandback', -600, -300);
setScrollFactor('gm_heartlandback', 0.9, 0.9);
makeLuaSprite('gm_heartlandfront', 'gm_heartlandfront', -600, -300);
setScrollFactor('gm_heartlandfront', 0.9, 0.9);

makeLuaSprite('gm_heartlandcurtains', 'gm_heartlandcurtains', -600, -300);
setScrollFactor('gm_heartlandcurtains', 0.9, 0.9);

end
addLuaSprite('gm_heartlandback', false);
addLuaSprite('gm_heartlandfront', false);
addLuaSprite('gm_heartlandcurtains', false);

Are you modding a build from source or with Lua?
Lua
What is your build target?
Windows x64
Did you edit anything in this build? If so, mention or summarize your changes.
No

Why is addLuaSprite outside of onCreate

Why is addLuaSprite outside of onCreate

well I just fixed it by moving the addLuaSprite into the side of onCreate, so uh thanks for help


[BUG] flyteadmin db models seem to have broken gorm indices

Describe the bug
Not sure I fully understand how gorm works, but according to https://gorm.io/docs/indexes.html, this line doesn't seem to be correct: https://github.com/lyft/flyteadmin/blob/master/pkg/repositories/models/task.go#L8
There are a few places having the same issue.
Expected behavior
index: is properly prefixed.
Flyte component

[ ] Overall
[ ] Flyte Setup and Installation scripts
[ ] Flyte Documentation
[ ] Flyte communication (slack/email etc)
[ ] FlytePropeller
[ ] FlyteIDL (Flyte specification language)
[ ] Flytekit (Python SDK)
[x] FlyteAdmin (Control Plane service)
[ ] FlytePlugins
[ ] DataCatalog
[ ] FlyteStdlib (common libraries)
[ ] FlyteConsole (UI)
[ ] Other

To Reproduce
N/A
Screenshots
N/A
Environment
Flyte component

[ ] Sandbox (local or on one machine)
[ ] Cloud hosted

[ ] AWS
[ ] GCP
[ ] Azure

[ ] Baremetal
[ ] Other

Additional context
N/A

Fixed by https://github.com/lyft/flyteadmin/pull/134

thank you!!


Repository naming

Username-labs could potentially cause an issue if that repo already exists on the account.
For example:
We have
Organization/team: One
User: Destidom
Repo: Destidom-lab

We try to add a new repo to another organization.
Organization/team: Two
User: Destidom
Repo: Destidom-lab

This will not be allowed, since it already has a repo called Test-lab. So I suggest adding the course name too in the repo name, something like below.
Destidom-<coursename>-lab

Or should we use another naming solution for user repositories?

We found out this is not a problem.


⚠ SceneTime has degraded performance

In 4ed1b9b, SceneTime ($ST) experienced degraded performance:

HTTP code: 200
Response time: 1556 ms

Resolved: SceneTime performance has improved in 4e3aa05 after 6 minutes.


Update default styles and polish focus treatment

This PR address some of the issues from #109549

Updates default styles
Removes cell selection bg (color token is still there)
Updates cell focus color to use focusBorder color and creates a new token notebook.selectedCellBorder
Makes cell toolbar appear on hover instead of on focus (means only one toolbar is shown at a given time)
Adds a placeholder text in status bar when cell is focused (this needs to be localized)
Increased hit target for run and fold icons

Merging this so we can test out the styles.


Updating instructions for newer exotic-ld versions

This PR resolves #540
I've set a lower limit on the exotic-ld version to 3.1.2 which is their latest version (contains the updated post commissioning sensitivity files for JWST), and I've updated the documentation to make sure folks know how to download those reference files and know that the reference files change with different ExoTiC-LD reference files.

Just realized this is failing because the ExoTiC-LD code (on pip and GitHub) only has version 3.0.0 while the reference files are on 3.1.2. I've sent David and Hannah an email to make sure we're supposed to use those two together; Hannah is out of the office for a while, so hopefully David responds fairly soon. Either way, I'll have to tweak some of the files and wording to make that clearer in these instructions.


Typo on raft.md page

Fix #2205

Proposed changes

Unreleased project version (optional)

Related issues (optional)

Thanks, @scjane :)


EmbeddedKafkaRule does not delete temporary directories

Hi,
I've faced the problem that EmbeddedKafkaRule does not delete temporary directories. Here is a simple example https://github.com/apanasevich/embeded-kafka-rule
Every run of EmbededKafkaTest#example on Windows leaves non-empty directory with zookeeper log file in user's temporary directory.

Closed as Won't Fix.
See mentioned above Apache Kafka JIRAs.
Should be addressed automatically after upgrade into a Kafka Client version with the fix.


OverlapCallback: copy constructor

Fix resolves a compilation error on Xcode: copy constructor is needed in line 202, 207.
Return-Value-Optimization is an optimization and doesn't exclude to have a default copy constructor.

Thanks a lot for your fix. I have merged this into the develop branch. This will be available in the next release of the library.


Chore: Reduce/remove issue templates

Description
I almost never fill out the other sections. And it can add friction for others opening up issues.

What This Enables

Impacts

Sounds good


[Event request] codeunit 5920 ServItemManagement - DeleteServItemOnSaleCreditMemo

Hello,
We need this handled event:

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. We will update this issue with information about availability.


Add computation of leftSingularMatrix to distributed SVD

add an extra step (Distributed) to corresponding entry in wrappers.py:has_dist
Most work will be adding the new pattern

use something like map-reduce-star-plus if same can be used for QR(#49)
otherwise follow dist_custom pattern

add using leftSingularMatrix to svd examples

implemented


Feature: Ssh Agent as authentication method

I really like the project but would you consider adding ssh Agent as an authentication method?

Hi @DonRico,
The Jsch library seems to support ssh-agent. I may work on this when I have some spare time.

Cool, no pressure. :)


command-line wrapper task for Continuous Integration

Hi Michael,
Thanks for the nice gem.  I've added a rake task that wraps guard-jasmine for continuous integration environments.  It starts/stop a server, executes guard-jasmine against it, then parses the output and returns success or failure so CI can succeed/fail the build accordingly.   The parsed output looks similar to how RSpec does it.
I thought I'd pass it along, in case you think others might find it valuable.
Cheers,
-Michael Kintzer

Thanks for the pull request. I looks to me that the Rake task duplicates existing functionality from the CLI tool guard-jasmine. What's the difference to the already supplied guard-jasmine? I see the addition of passenger, but wouldn't it better to integrate passenger into the server class, so people can use it also for TDD development?

Thanks for the reply.  Maybe I'm missing something, but the guard-jasmine tool isn't starting a server AFAIK, and one is required for jasmine to execute.  Also, guard-jasmine always returns a 0 exit code whether or not it succeeds.
The task I wrote allows a CI server to start up a rails server, executes jasmine against it (using guard-jasmine), parses the guard-jasmine output and choose whether or not to fail the build based upon whether or not jasmine tests failed.
The passenger bit, is just a convenience in case mongrel or webrick is not available.
HTH.   If you don't find it useful, that's fine.

In the early days of guard-jasmine, the guard-jasmine bin was a simple debug tool that just queried an already running server and printed the raw JSON output from the runner. This was changed in a later version (blame on me for not writing a CHANGELOG), where this simple debug helper was renamed to guard-jasmine-debug and a new CI helper gaurd-jasmine was introduced.
The guard-jasmine CI helper will indeed start a server, run the tests and stop the sever again. It can be configured through the cli options:
Usage:
  guard-jasmine spec

Options:
  -s, [--server=SERVER]          # Server to start, either `auto`, `webrick`, `mongrel`, `thin`, `jasmine_gem` or `none`
                                 # Default: auto
  -p, [--port=N]                 # Server port to use
                                 # Default: 8888
  -u, [--url=URL]                # The url of the Jasmine test runner
                                 # Default: http://<IP_ADDRESS>:8888/jasmine
  -b, [--bin=BIN]                # The location of the PhantomJS binary
                                 # Default: /usr/local/bin/phantomjs
  -t, [--timeout=N]              # The maximum time in milliseconds to wait for the spec runner to finish
                                 # Default: 10000
  -c, [--console=CONSOLE]        # Whether to show console.log statements in the spec runner, either `always`, `never` or `failure`
                                 # Default: failure
  -e, [--server-env=SERVER_ENV]  # The server environment to use, for example `development`, `test` etc.
                                 # Default: development

Run the Jasmine spec runner

The CI helper will also have a different exit code depending on the result: without failing specs the exit code is 0, whereas with failing specs the exit code is 1.
It has nothing to do with the usefulness of the Rake task, it has to do with code duplication and spec coverage :P

Thank you. I really appreciate any kind of contribution.


Zoom using scroll + mouse/scroll wheel

Is your feature request related to a problem? Please describe.
No.
Describe the solution you'd like
To be able to zoom in and out using ctrl + mouse/scroll wheel.
Describe alternatives you've considered
I can already zoom in and out with ctrl + +/-.
Additional context
OS: Windows 10.

Added to roadmap

The simplest way to do this is via the new Electron API which only works on windows atm (https://github.com/electron/electron/pull/17747). Still needs testing but should work.


Add implementation permission parser

Like a https://github.com/facundoolano/google-play-scraper/blob/dev/lib/permissions.js
Maybe I'll do in late October 🤷♂

Awesome, thanks!

Awesome, thanks!

I think the scraper of danieliu works very well, but I also miss the opportunity to scrape permissions. So I wrote my own script for this. Perhaps it can also help you. It's a small Python 3 script: ScrapePermissions.py.


Datepicker feature request: Ability to specify time intervals

I would personally like the ability to specify the minute intervals for the datetimepicker, as there are a lot of use-cases where we only need the time rounded to the nearest 5 minute interval.
Most events (i.e. meetings, classes, conferences) will start at a time rounded to the nearest 5. In this case, we do not care for minutes 1-4 & 6-9.
Maybe something along the lines of:
<md2-datepicker
     timeInterval="5">
</md2-datepicker>

where timeInterval is either minutely or 5-minutely.

+1


Add ValueConvertible conformance to Swift Bool

Addresses https://github.com/Zewo/Zewo/issues/222

Sounds good. I'll address those now.

Awesome! Waiting for travis to pass and I'll merge!

@Danappelxx I've discovered today that this implementation will not work with a postgres boolean field, since the buffer is returned as either "t" or "f".
Would you entertain an implementation similar to this:
extension Bool: ValueConvertible {
    public init(rawSQLData buffer: Buffer) throws {
        // read int, 0 = false, 1 = true
        let value = try Int(rawSQLData: Bool.sanitizedBooleanBuffer(rawBuffer: buffer))

        switch value {
        case 0: self = false
        case 1: self = true
        default: throw ValueError(description: "Failed to convert data to Bool. Excepted 0/1, got \(value).")
        }
    }

    public var sqlValue: Value {
        return (self ? 1 : 0).sqlValue
    }

    private static func sanitizedBooleanBuffer(rawBuffer: Buffer) -> Buffer {
        guard let bufferPrefix = rawBuffer.first else {
            return rawBuffer
        }
        let hex0: UInt8 = 0x30
        let hex1: UInt8 = 0x31
        let hexLowercaseT: UInt8 = 0x74
        let hexLowercaseF: UInt8 = 0x66

        switch bufferPrefix {
        case hexLowercaseT:
            return Buffer("1")
        case hexLowercaseF:
            return Buffer("0")
        case hex0, hex1:
            break
        default:
            break
        }
        return rawBuffer
    }
}

This works well but I could see the concern of being to database specific within the SQL framework. That being said it would be tricky to "override" this in the postgres framework since this is just an extension on bool that would collide if we tried to implement it over in https://github.com/Zewo/PostgreSQL.
Thoughts?


Added support for app-copyright, app-version and version-string

See #226.

Used the same keys like electron-packager ('app-version' instead of appVersion).
Currently only support the Nativefier programmatic API

Really sorry for the delay. I've added some comments regarding the object keys, and could you add some documentation as well? Thanks!

I'm sorry for my delay! ..didn't get any notifications about your comments.
I will update my PR! :+1:

This PR was inadvertently deleted due to us getting rid of the old development branch (we came back to merging on good'ol master). Changing base branch to master and reopening.

@Purii rebase welcome, if you have the time for that.

Closing as superseded by https://github.com/jiahaog/nativefier/pull/244


Enable hyperparameter tuning with NNI, fix dependency error in testing_CI, and update docs

What does this PR do?

fixing #187;
fixing #222;
updating docs;

Before submitting

[x] This PR is made to fix a typo or improve the docs (you can dismiss the other checks if this is the case).
[x] Was this discussed/approved via a GitHub issue? Please add a link to it if that's the case.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have written necessary tests and already run them locally.

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

38 of 110 (34.55%)  changed or added relevant lines in 12 files are covered.
4 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-1.0%) to 84.111%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

pypots/cli/pypots_cli.py
0
2
0.0%

pypots/classification/base.py
7
12
58.33%

pypots/clustering/crli/model.py
7
12
58.33%

pypots/clustering/vader/model.py
7
12
58.33%

pypots/clustering/base.py
5
12
41.67%

pypots/forecasting/base.py
5
12
41.67%

pypots/cli/tuning.py
0
41
0.0%

Files with Coverage Reduction
New Missed Lines
%

pypots/cli/env.py
1
65.31%

pypots/data/base.py
1
80.77%

pypots/base.py
2
70.55%

Totals

Change from base Build<PHONE_NUMBER>:
-1.0%

Covered Lines:
4378

Relevant Lines:
5205

💛  - Coveralls


Add a theme switcher utility component for Codesandbox

Theme switcher
Brief description
Adds a theme switcher utility component for our code sandbox. The design will be based on this prototype.
Links to other materials
Prototype: https://github.ibm.com/Alexander-Melo/carbon-theme-switcher

related to #514 ?

@dcwarwick


Word wrapping on non-monospace still fails

VSCode Version: 1.42.0-insider
OS Version: macOS 10.13.6

@alexdima
I was the one who reported #2654 many moons ago. The purported fix at #89389 does not appear to address the problem of word wrap occurring prematurely when a proportional font is used:

Relevant configuration settings from my settings.json file:

As can be seen, the new editor.wrappingAlgorithm setting does not appear to be recognized, which would suggest I either don't have the latest version of VS Code, or have some misconfiguration, but I'm out of ideas in this regard.
Open Sans Code is a derivation of the Open Sans font that I tuned for proportional font coding. I've swapped this out for other proportional fonts (Lato, Helvetica) with no success.
I also attempted to pass these settings directly to a test project that uses the Monaco editor with similar results.

Does this issue occur when all extensions are disabled?: Yes

Per #89573 it got renamed editor.wrappingStrategy


API endpoints appear to have broken

Looks like the API is on v3 while the network routes are still pointing to v1:
https://github.com/driven-app/porsche-connect/blob/447606fee91f71d0b739f0bed73b34af5144aa5e/Sources/PorscheConnect/NetworkRoutes.swift#L13

Looks like the new initial auth URL is https://login.porsche.com/as/authorization.oauth2

fwiw the python library appears to be able to auth and hit endpoints: https://github.com/CJNE/pyporscheconnectapi

Ignore the first two comments because it seems like the authentication flow is basically magic 😂

Hi @jverkoey – I hope the PR I pushed to main just there resolves this issue. Its been a while since I touch this library, so bumped the library dependencies and updated some code conventions that my code style checker threw up.
I've tagged the latest with release v0.1.13 – in case you just want to use this as a dependent package in your own app.
Let me know how you get on and I'll close this issue.


Unbreaking 5?

I could probably find this somewhere else but on this server i play on this player somehow got a unbreaking 5 pickaxe after a staff member left and claim its a feature of crazy enchants i tried opening like 2 inventories of legendary books and not one of them was unbreaking 5 is this some staff abuse or is it a feature.
Probably the wrong spot but idc

Just got an email notification for this 1 1/2 years later


Correction in custom validation example of Guide.md

If only Full name is allowed then error should be thrown when the value is not equal to 'Full name'.

Thanks Pranay!


Refactor to Hub

This should allow management of data to be done in the Hub object, and factored out of Graph