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

Select a windows reference module #20509

Closed
wants to merge 1 commit into from

Conversation

dagwieers
Copy link
Contributor

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

win_service

ANSIBLE VERSION

v2.3

SUMMARY

As discussed in #20505 we would like to point from the documentation to
a module we choose as the reference module for Windows.

I picked win_service and cleaned it up to the best of my abilities.
I also added check-mode support as an example.

Let the bikeshedding begin !

@dagwieers
Copy link
Contributor Author

dagwieers commented Jan 20, 2017

So, let me start off by pointing out that:

  • This module does not have any -type set for any parameter. (Fixed with win_service: support for Automatic (Delayed Start) #20124)
  • The module does not have any extended documentation, but let us first agree on the module/logic/semantics first and then add the proper code comments.
  • This PR will also include documentation updates pointing to this module, as well as providing a list of best practices (including pointing to powershell.ps1 to see what functionality is already provided).

@ansibot
Copy link
Contributor

ansibot commented Jan 20, 2017

@ansibot ansibot added affects_2.3 This issue/PR affects Ansible v2.3 core_review In order to be merged, this PR must follow the core review workflow. docs_pull_request module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. windows Windows community labels Jan 20, 2017
@jctanner jctanner added windows Windows community and removed windows Windows community needs_triage Needs a first human triage before being processed. labels Jan 20, 2017
@dagwieers dagwieers force-pushed the win-reference branch 2 times, most recently from 8d373c7 to 967e5ae Compare January 21, 2017 20:14
@ansibot ansibot added the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label Jan 25, 2017
@mattclay

This comment has been minimized.

@mattclay mattclay closed this Jan 26, 2017
@mattclay mattclay reopened this Jan 26, 2017
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. labels Jan 26, 2017
@dagwieers dagwieers force-pushed the win-reference branch 3 times, most recently from b5770fe to 67f9b42 Compare January 26, 2017 10:34
@dagwieers

This comment has been minimized.

@dagwieers dagwieers changed the title Select win_service as the reference windows module Select a windows reference module Jan 26, 2017
@mattclay

This comment has been minimized.

@gundalow
Copy link
Contributor

win_service integration tests are failing, needs_revision

@dagwieers

This comment has been minimized.

As discussed in ansible#20505 we would like to point from the documentation to
a module we choose as the reference module for Windows.

I picked win_service and cleaned it up to the best of my abilities.
I also added check-mode support as an example.

Let the bikeshedding begin !
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 1, 2017
@mattclay

This comment has been minimized.

@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Feb 2, 2017
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Feb 2, 2017
@jborean93
Copy link
Contributor

jborean93 commented Feb 3, 2017

@dagwieers @ilpianista I was hoping to get all out changes to win_service in before the next release but it seems like we have 3 different pull requests going on at the same time;

I am happy to modify my pull request to add in all your changes instead of trying to make 3 different changes ending up in some merge issues but wanted to get your thoughts first. Are you happy for me to do so or would we just want to get a free for all :)

@dagwieers dagwieers closed this Feb 4, 2017
@dagwieers dagwieers reopened this Feb 4, 2017
@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Feb 6, 2017
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Feb 6, 2017
@dagwieers
Copy link
Contributor Author

@jborean93 You are right after reviewing #19143 it makes no sense to keep this around. I'll be reviewing your PR.

@dagwieers dagwieers closed this Feb 20, 2017
@dagwieers dagwieers deleted the win-reference branch February 20, 2017 11:37
@dagwieers dagwieers mentioned this pull request Feb 20, 2017
@ansibot ansibot added docs This issue/PR relates to or includes documentation. and removed docs_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.3 This issue/PR affects Ansible v2.3 ci_verified Changes made in this PR are causing tests to fail. docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. windows Windows community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants