Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Fix unchecked "application" attribute on Get-WebVirtualDirectory. #1354

Closed
wants to merge 2 commits into from
Closed

Fix unchecked "application" attribute on Get-WebVirtualDirectory. #1354

wants to merge 2 commits into from

Conversation

mather
Copy link

@mather mather commented Dec 10, 2015

It always raise error if I specify "application" attribute, because "application" parameter is not passed on "Get-WebVirtualDirectory" function.

It always raise error if I specify "application" attribute, because "application" parameter is not passed on "Get-WebVirtualDirectory" function.
@gregdek
Copy link
Contributor

gregdek commented Dec 10, 2015

Thanks @mather. @WHenrik please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with text 'shipit' or 'needs_revision' as appropriate.

@robynbergeron
Copy link
Contributor

@WHenrik This change is still pending your review; do you have time to take a look and comment? Please comment with text 'shipit' or 'needs_revision' as appropriate.

@gregdek
Copy link
Contributor

gregdek commented Jan 20, 2016

@WHenrik still waiting on your review. Please comment with text 'shipit' or 'needs_revision' as appropriate. If we don't hear from you within 14 days, we will start to look for additional maintainers for this module.

@gregdek
Copy link
Contributor

gregdek commented Feb 8, 2016

Assigning to @ansible/core review. Thanks for your patience @mather.

@@ -102,7 +102,12 @@ try {
$result.changed = $true
}

$directory = Get-WebVirtualDirectory -Site $site -Name $name
$directory = if($application) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but can you follow the arg splat pattern used above on New-WebVirtualDirectory instead of repeating the command?

@gregdek
Copy link
Contributor

gregdek commented Mar 29, 2016

@mather A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes.

[This message brought to you by your friendly Ansibull-bot.]

@mather
Copy link
Author

mather commented Mar 30, 2016

I have lost my testing environment for this PR.
So I just tried to modify as you commented, but no test by my side, sorry.

@gregdek
Copy link
Contributor

gregdek commented Apr 6, 2016

Thanks @mather. Unfortunately, we can't really put this into review until you can test. If you can't actually test the change you're proposing, I advise you to just close it.

If you are able to test this change and it passes, then please comment "ready_for_review". Thanks.

@gregdek
Copy link
Contributor

gregdek commented Apr 22, 2016

@mather A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer.

[This message brought to you by your friendly Ansibull-bot.]

@mather
Copy link
Author

mather commented Apr 28, 2016

Sorry, I can't follow this issue anymore.

@mather mather closed this Apr 28, 2016
@mather mather deleted the fix_win_iis_virtualdirectory branch April 28, 2016 13:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants