Added xSPWeb resource.

[wulfland]

I added a resource to provision subwebs to site collections with the most common parameters.

---

This change is

[BrianFarnhill]

Reviewed 4 of 5 files at r2.
Review status: all files reviewed at latest revision, 11 unresolved discussions.

---

Modules/xSharePoint/DSCResources/MSFT_xSPWeb/MSFT_xSPWeb.psm1, line 8 [r2] (raw file):
Can you add the parameter attribute here to specifically mark it as not required like the other properties?

---

Modules/xSharePoint/DSCResources/MSFT_xSPWeb/MSFT_xSPWeb.psm1, line 74 [r2] (raw file):
It looks like $result is not ever used here so we don't need to have the "$result =" bit at the front of this line

---

Modules/xSharePoint/DSCResources/MSFT_xSPWeb/MSFT_xSPWeb.psm1, line 78 [r2] (raw file):
A couple of issues with this part - if you are using PsDscRunAsCredential in PowerShell 5 instead of InstallAccount (which is our recommendation) then this will fail. Also I've got something on my radar to add this to xSPWebApplication to list the accounts that should have the process identity rights, so lets take this out of this resource and I'll prioritise up that other extension to cover this scenario.

---

Modules/xSharePoint/DSCResources/MSFT_xSPWeb/MSFT_xSPWeb.psm1, line 85 [r2] (raw file):
Ensure is optional so this will throw an exception if it isn't specifically provided. In some of our other resources we are adding it to the $PSBoundParameters collection before we hit the Invoke-xSharePointCommand line so we always know it's there inside that block. We do that with:

$PSBoundParameters.Ensure = $Ensure

---

Modules/xSharePoint/DSCResources/MSFT_xSPWeb/MSFT_xSPWeb.psm1, line 91 [r2] (raw file):
Same comment here about Ensure being optional - fixing the above reference will cover you here as well though

---

Modules/xSharePoint/DSCResources/MSFT_xSPWeb/MSFT_xSPWeb.psm1, line 97 [r2] (raw file):
Have you tested this where you see two or more of your options changed in one go? I'm sure I've seen scenarios where calling $web.Update() more than once could lead to issues. Either way couldn't we call it once after the if statements so we only write back to SQL once instead of potentially 4 times?

---

Modules/xSharePoint/DSCResources/MSFT_xSPWeb/MSFT_xSPWeb.psm1, line 143 [r2] (raw file):
You should be testing Ensure as well to see if the web should be removed. Also you'll need to do that same trick to add it to $PSBoundParameters as mentioned above to ensure that the test method we use always sees a value, even when one isn't supplied.

---

Modules/xSharePoint/DSCResources/MSFT_xSPWeb/MSFT_xSPWeb.schema.mof, line 25 [r2] (raw file):
You need to add descriptions to each of these properties - have a look at all of our other resources to see the formatting.

---

Tests/xSharePoint/xSharePoint.xSPWeb.Tests.ps1, line 36 [r2] (raw file):
When mocking something generic like New-Object, can you add the parameters check to it so that we ensure we only return that object when a specific type is asked for?

Mock New-Object { ... } -ParameterFilter { $TypeName -eq "Microsoft.SharePoint.SPSite" }

---

Tests/xSharePoint/xSharePoint.xSPWeb.Tests.ps1, line 113 [r2] (raw file):
We can move this mock to the top of the describe block where you mock the New-Object call as we won't change this mock at all between the contexts

---

Tests/xSharePoint/xSharePoint.xSPWeb.Tests.ps1, line 135 [r2] (raw file):
Can you move this declaration of $web in to the Mock statement rather than leave it in the context here? Then it will stay at its correct scope

---

Comments from Reviewable

[wulfland]

Reviewable termintes with an exception. Can I do something more?

[BrianFarnhill]

Just the one point about the install account left to handle on this one

---

Reviewed 4 of 4 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[wulfland]

I checked in the 'Invoke-xSharePointCommand' function. Eather the credential is present or the user name is correct:

if ($null -eq $Credential) {
    if ($Env:USERNAME.Contains("$")) {
        throw [Exception] "You need to specify a value for either InstallAccount or PsDscRunAsCredential."
        return
    }
} 

I now check if the credential is null and use the $env:UserName instead.

[wulfland]

This is still labeled "Waiting on author". Anything more for me to do?

[BrianFarnhill]

@wulfland Sorry dude - we had a holiday weekend and I've been busy. Finalising this review is on my list for later today.

[BrianFarnhill]

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

---

Comments from Reviewable