WebAdministrationDsc
WebAdministrationDsc copied to clipboard
Converting PHP-FastCGI handler from a managed handler to a module mapping
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.
DSCResources/MSFT_xIisModule/MSFT_xIisModule.psm1, line 161 at r1 (raw file):
Modules = $ModuleType ScriptProcessor = $Path ResourceType = 'File'
Why hardcoding this to File? I suggest leaving it out or adding the parameter $ResourceType.
Comments from Reviewable
I'm about to make bigger changes to IISModule (and other resources), so I have added a comment to Reviewable. I'd like this issue to be Done before I start.
I can resolve my comment myself too, as the author hasn't been active on this for a few months now. Is it okay to pass the review then? Asking because that's usually a bad practice and I don't want to just mess up the system. I will make the resource use a new parameter as I suggested in my comment as part of the bigger changes mentioned before.
Just to make sure I'm following you want to basically incorporate this change into your PR? Personally I'm fine with whatever as I don't fully comprehend this change.
Exactly. The changes the author meant to perform was changing "Module" to "Modules" (with an S), which fixes a bug. However, author also added a hardcoded ResourceType, which is kind of dirty and as part of my changes I'd add a Parameter for setting the ResourceType. The question if I could pass the "dirty" change as I'll be soon replacing that anyway.
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.
Comments from Reviewable
I'm fine with you doing the change in yours and closing this one. Let's give another day or two for @andy1547 to respond before moving forward. If he doesn't respond I'll close this PR.
@Devvox93 I agree that ResourceType should be a string parameter with a ValidationSet of Unspecified (don't think this is an option in the UI but is in handler documentation), File, Directory and Either. The IIS UI does not force the user to input this and defaults to File, so I'd suggest using that as the default. Would also need to add ResourceType to GetScript and validate in TestScript. @tysonjhayes I won't be able to do these changes any time soon so by all means supersede them, however for the time being I'd suggest keeping this PR open.
With the UI defaulting to File (I didn't think about that), I suggest this PR to be approved, as it does fix the bug mentioned in issue #305 . The extended functionality of using a Parameter (also in Get and Test) can be added later in my PR.
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.
Comments from Reviewable
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.
DSCResources/MSFT_xIisModule/MSFT_xIisModule.psm1, line 161 at r1 (raw file):
Previously, Devvox93 wrote…
Why hardcoding this to File? I suggest leaving it out or adding the parameter $ResourceType.
Default value that UI sets is File, so it's not that ugly. Future change will use a parameter.
Comments from Reviewable
Reviewed 1 of 1 files at r1. Review status: all files reviewed at latest revision, all discussions resolved.
Comments from Reviewable
@Devvox93 you solution sounds good. Do you have a PR ready for this change? If that includes this change then I suggest we close this PR. @andy1547 I will set this a abandoned, but if you want to continue the work on this than, then please let me know.