Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support null value for ServiceAutoStartProvider property of xWebApplication #229

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fredgate
Copy link

@fredgate fredgate commented Sep 8, 2016

I open this pull request to follow my work to fix the issue #225.

The goal is that xWebApplication resource provide the avibility to remove ServiceAutoStartProvider already configured for a web application in IIS.
I actually remove the property for the web application. Could I have a feedback (about logic, readability...) on the pushed commit ?
I think that a unit test should be added to validate this case, but I don't know exactly what and how to do. Some help for this shoud be appreciate.

Surely should we also remove the provider of the web application from IIS's providers list if no other application references this provider. What do you think ?


This change is Reviewable

@fredgate fredgate force-pushed the removeServiceAutoStartProvider branch from c282efe to 18e20b0 Compare September 8, 2016 12:13
@mbreakey3 mbreakey3 added the help wanted The issue is up for grabs for anyone in the community. label Sep 8, 2016
@mbreakey3
Copy link
Contributor

Hi @fredgate - Yes, a unit test would be great. If you want to check out our TestGuidelines that should get you started. You can just add it in to the existing xWebApplication tests. Thanks!

@mbreakey3 mbreakey3 added the waiting for author response The pull request is waiting for the author to respond to comments in the pull request. label Oct 18, 2016
@nzspambot
Copy link

Would be nice also to be able to clear /system.applicationHost/serviceAutoStartProviders as well but it's not really needed tbh

@tysonjhayes
Copy link

Hi @fredgate are you still working on this? Happy to help if needed.

@rosberglinhares
Copy link
Contributor

rosberglinhares commented Apr 14, 2017

Passing ServiceAutoStartProvider = $null will result in $PSBoundParameters.ContainsKey('ServiceAutoStartProvider') evaluating to $false. Adding the AllowNull() attribute doesn't work too, so I think we have to use another approach. May be empty string? Is passing an empty string to remove an item in compliance with the Powershell guidelines?

xWebApplication WebApp
{
    Name = "Foo"
    Ensure = "Present"
    Website = "Default Web Site"
    WebAppPool = "Foo"
    PhysicalPath = "c:\inetpub\foo"
    ServiceAutoStartEnabled = $false
    ServiceAutoStartProvider = ''
}

@johlju
Copy link
Member

johlju commented Apr 23, 2018

@fredgate are you still able to work on this pull request? If so I can help you review it and get it merged. There are merge conflicts and we should have tests that test this scenario. If you can't work on this pull request, no worries, then I will label it as abandoned so that someone else can take over the work. 🙂

@johlju johlju removed the help wanted The issue is up for grabs for anyone in the community. label Apr 23, 2018
@johlju
Copy link
Member

johlju commented May 23, 2018

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.

@johlju johlju added abandoned The pull request has been abandoned. and removed waiting for author response The pull request is waiting for the author to respond to comments in the pull request. labels May 23, 2018
@johlju johlju changed the base branch from dev to master December 30, 2019 12:20
@johlju johlju changed the base branch from master to main January 7, 2021 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned The pull request has been abandoned.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants