WebAdministrationDsc
WebAdministrationDsc copied to clipboard
dsccommunity
xWebfarm DSCResource
This PR is for the xWebfarm (http://www.iis.net/downloads/microsoft/web-farm-framework) DSC resource. The resource tries to be as identical to the XML as possible.
Example: https://github.com/xunilrj/xWebAdministration/blob/dev/Examples/Sample_xWebfarm_NewWebfarm.ps1
To understand all the functionalities implemented I suggest to look to the tests.
https://github.com/xunilrj/xWebAdministration/blob/dev/Tests/Unit/MSFT_xWebfarm.tests.ps1
This change is 
Hi @xunilrj, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.
TTYL, MSBOT;
@xunilrj, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.
Thanks, MSBOT;
Sorry I'm just now having time to review this. Couple of things. Please update the resource to follow all StyleGuidelines. Specifically curly braces get their own line, and all functions should be noun-verb (and preferably with an approved verb).
Please make use of LocalizedData, specifically move all of the verbose message strings to it, it allows localized a central location for the strings, and allows targeting loc changes if we ever decide to make them.
I would also suggest using the following style for verbose messages:
Write-Verbose -Message ($LocalizedData.VerboseSetTargetUpdatedApplicationPool -f $Name, $ApplicationPool)
For Error Messages please use the New-TerminatingError from the Helper.psm1 module in the root of the DSCResource Folder
While the testing you have is pretty good, would it be possible for you to write some integration tests as well?
Review status: 0 of 4 files reviewed at latest revision, 13 unresolved discussions.
DSCResources/MSFT_xWebfarm/MSFT_xWebfarm.psm1, line 35 [r2] (raw file): I'm not actually sure this works in DSC, and given that we're calling it from a dsc config I'm not sure I see the value of having it.
DSCResources/MSFT_xWebfarm/MSFT_xWebfarm.psm1, line 255 [r2] (raw file): I would suggest avoiding declartions such as these, instead I would suggest language like "More then one webfarm found, please investigate the config file located at $location"
DSCResources/MSFT_xWebfarm/MSFT_xWebfarm.psm1, line 263 [r2] (raw file): Please use the param construct
DSCResources/MSFT_xWebfarm/MSFT_xWebfarm.psm1, line 316 [r2] (raw file): Please put all parameters on their own lines.
ie:
param
(
[string] $ConfigPath,
[xml] $XML
)
DSCResources/MSFT_xWebfarm/MSFT_xWebfarm.psm1, line 327 [r2] (raw file): All files must have new lines at the end of them, please adjust.
DSCResources/MSFT_xWebfarm/MSFT_xWebfarm.schema.mof, line 4 [r2] (raw file): Best practice would be to add Description fields here.
Examples/Sample_xWebfarm_NewWebfarm.ps1, line 1 [r2] (raw file): I'm not sure this is necessary here.
Examples/Sample_xWebfarm_NewWebfarm.ps1, line 3 [r2] (raw file): This shouldn't be ncessary.
Examples/Sample_xWebfarm_NewWebfarm.ps1, line 31 [r2] (raw file): Typically in our examples we do not actually configure the machine, we just provide the configuration listed above. Please use our other examples as guidelines.
Tests/Unit/MSFT_xWebfarm.tests.ps1, line 1 [r2] (raw file): Please remove these comments as they are not necessary once the template has been copied.
Tests/Unit/MSFT_xWebfarm.tests.ps1, line 15 [r2] (raw file): You can remove these comments too.
Tests/Unit/MSFT_xWebfarm.tests.ps1, line 675 [r2] (raw file): Please test your internal functions as well.
Tests/Unit/MSFT_xWebfarm.tests.ps1, line 687 [r2] (raw file): New line here please.
Comments from Reviewable
Hi, haven't heard from you in a while, as it's been 18 days since I last pinged you and I haven't seen any activity I'm going to go ahead and close this. Please feel free to reopen it once you've made some more changes.
Thanks!
I will reopen this and mark this as abandoned so someone else can continue this work, since this is work on an open issue.
