Missing settings for xWebAppPoolDefaults

[onurbiyik]

Fixes #105.

This is mostly based on existing xWebAppPool code. There is some code duplication introduced between these two resources with this change, which means there is room for refactoring.

---

This change is 

[onurbiyik]

Integration tests need to be updated. I should get back working on it.

[johlju]

@onurbiyik Impressive work here! I will review this one for you, I hope you still can work on this?

[onurbiyik]

@johlju Thank you. I can work on this but probably not very soon.

I appreciate if you can review this.

Note that most changes were based on how xWebAppPool code looked by the time I was working on it. So, any changes committed since then should be incorporated to xWebAppPoolDefaults as well.

[johlju]

@onurbiyik I can review it.

Regarding changes to xWebAppPool. If both share a lot of code, we should start by adding helper functions for those new changes. Otherwise xWebAppPoolDefaults resource will be left behind quickly. If changes to xWebAppPool was done in a helper function, tests of xWebAppPoolDefaults would help make sure those changes didn't break xWebAppPoolDefaults, and vice verse.

[johlju]

This was a big one to review, took a while :) Again, impressive work.

---

Reviewed 3 of 5 files at r1, 4 of 4 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

---

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

autoStart

We should use PascalCase on all these parameter names. For example 'AutoStart' (upper 'A'). Throughout this resource parameters.

---

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

autoStart

We should use PascalCase on all these parameter names. For example 'AutoStart' (upper 'A'). Throughout this example.

---

DSCResources/MSFT_xWebAppPoolDefaults/MSFT_xWebAppPoolDefaults.psm1, line 29 at r2 (raw file):

Name = 'autoStart'

We should use PascalCase on all these parameter names. For example Name = 'AutoStart' (upper 'A'). Throughout this file.

---

DSCResources/MSFT_xWebAppPoolDefaults/MSFT_xWebAppPoolDefaults.psm1, line 149 at r2 (raw file):

function Set-TargetResource {
    <#

Please move this comment-based help block above the function Set-TargetResource {-statement.

---

DSCResources/MSFT_xWebAppPoolDefaults/MSFT_xWebAppPoolDefaults.psm1, line 169 at r2 (raw file):

        [Boolean] $autoStart,

Please add [Parameter()] for each of the parameters in Set-TargetResource and Test-TargetResource (see the parameter $ManagedRuntimeVersion as an example how the parameter should be formatted. Throughout.

---

DSCResources/MSFT_xWebAppPoolDefaults/MSFT_xWebAppPoolDefaults.psm1, line 169 at r2 (raw file):

$autoStart

We should use PascalCase on all these parameter names. For example $AutoStart (upper 'A'). Throughout this file, both Set-TargetResource and Test-TargetResource

---

DSCResources/MSFT_xWebAppPoolDefaults/MSFT_xWebAppPoolDefaults.psm1, line 171 at r2 (raw file):

[String]

Please change to [System.String]. Throughout this file.

---

DSCResources/MSFT_xWebAppPoolDefaults/MSFT_xWebAppPoolDefaults.psm1, line 182 at r2 (raw file):

[Boolean]

Please change to [System.Boolean]. Throughout this file.

---

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

class MSFT_xWebAppPoolDefaults : OMI_BaseResource
{
    [Key, Description("Dummy value because we need a key, always 'Machine'"), ValueMap{"Machine"}, Values{"Machine"}] string ApplyTo;

We should use the single instance method here so it's cleare it can only be used once per configuration, and align with other resources that are alike.
https://docs.microsoft.com/en-us/powershell/dsc/singleinstance

Example of it here: https://github.com/PowerShell/xComputerManagement/blob/dev/Modules/xComputerManagement/DSCResources/MSFT_xPowerPlan/MSFT_xPowerPlan.schema.mof

---

DSCResources/MSFT_xWebAppPoolDefaults/MSFT_xWebAppPoolDefaults.schema.mof, line 7 at r2 (raw file):

autoStart

We should use PascalCase on all these property names. For example 'AutoStart' (upper 'A'). Throughout this file.

---

Examples/Sample_xWebAppPoolDefaults.ps1, line 25 at r2 (raw file):

autoStart

We should use PascalCase on all these parameter names. For example 'AutoStart' (upper 'A'). Throughout this file.

---

Tests/Integration/MSFT_xWebAppPoolDefaults.config.ps1, line 26 at r2 (raw file):

autoStart

We should use PascalCase on all these parameter names. For example 'AutoStart' (upper 'A'). Throughout this file.

---

Tests/Integration/MSFT_xWebAppPoolDefaults.Integration.Tests.ps1, line 86 at r2 (raw file):

                    Should Be $TestParameters['Credential'].GetNetworkCredential().Password
                }
                elseif ($parameter.Name -eq 'ApplyTo')

Couldn't we test this property just like any other?

---

Tests/Unit/MSFT_xWebAppPoolDefaults.tests.ps1, line 478 at r2 (raw file):

                It 'Should return True' {
                    Test-TargetResource -ApplyTo Machine @testParamsSplat |

We could splat all parameters if we are splatting. Throughout this file.

---

Comments from Reviewable

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (dev@d022234). Click here to learn what that means.
The diff coverage is 90%.

@@         Coverage Diff          @@
##             dev   #324   +/-   ##
====================================
  Coverage       ?    89%           
====================================
  Files          ?     14           
  Lines          ?   2354           
  Branches       ?      0           
====================================
  Hits           ?   2108           
  Misses         ?    246           
  Partials       ?      0

[johlju]

It seems you got commits from other contributors into this PR, but now worries, it is easy to resolve. To solve this you need to rebase your working branch using git rebase. Here is a link on how to rebase the working branch (same as resolving merge conflicts); https://github.com/PowerShell/DscResources/blob/master/GettingStartedWithGitHub.md#resolve-merge-conflicts
Let me know if you get stuck.

[johlju]

@onurbiyik Looks better now! There are still a conflicting file. Could you try the rebase again? Make sure to fetch from the "upstream" before doing rebase (steps in link in previous comment). Let me know if I can help! 🙂

[onurbiyik]

Thank you for keeping a close eye on this pr. That was helpful.

I need to rework on this issue - not just merely rebase and merge - and incorporate your review comments. It will take some time for me to get on it though.

[johlju]

@onurbiyik No worries. When you write 'Done' on the review comments I will be back to continue the review. 🙂 Let me know if you need any help before that.

[johlju]

Labeling this PR as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on the PR is taken up again.

[gstorme]

@onurbiyik are you still working on this?
If not, how can someone else take up on the review comments to proceed with this PR?

[regedit32]

Hi @gstorme - here's some info on how to pick up abandoned work: https://dsccommunity.org/guidelines/contributing/#how-to-continue-working-on-a-pull-request-pr-when-an-author-contributor-is-unable-to-complete-it