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.

Sign up for GitHub

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

Jump to bottom

Add docs on Windows module util requirements #76583

Merged
merged 2 commits into from
Dec 20, 2021
Merged

Add docs on Windows module util requirements #76583

merged 2 commits into from
Dec 20, 2021

Conversation

jborean93
Copy link
Contributor

SUMMARY

It was mentioned in IRC that the docs for the #AnsibleRequires statements were not as fleshed out as they could be. This adds the missing statements and includes more information for module utils in collections.

cc @briantist

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

Windows dev guide

@ansibot ansibot added affects_2.13 core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. docs_only All changes are to files within the docs/docsite/ directory docsite This issue/PR relates to the documentation website. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Dec 19, 2021
briantist
briantist approved these changes Dec 19, 2021
View reviewed changes
Copy link
Contributor

@briantist briantist left a comment

Choose a reason for hiding this comment

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

Brilliant! This is great, thanks Jordan!
cc @lowlydba

@samccann samccann removed the needs_triage Needs a first human triage before being processed. label Dec 20, 2021
@ansibot ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. 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 Dec 20, 2021
@samccann
Copy link
Contributor

@jborean93 I'd like to get this file moved into a collection instead of living in the ansible/ansible repo. Do you think it belongs in ansible.windows or community.windows?

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 20, 2021
@briantist
Copy link
Contributor

@jborean93 I'd like to get this file moved into a collection instead of living in the ansible/ansible repo. Do you think it belongs in ansible.windows or community.windows?

I personally think it belongs in ansible/ansible; while most of the Windows content itself moved to collections, the things mentioned here are central to ansible-core and its support for Windows.

Other collections that target Windows or contain content that works on Windows will come up (and we've been starting to recommend this to some folks who have put PRs in the existing Windows collections when their contribution would be hard to test or otherwise fit), so it seems strange to me to file this documentation under a specific collection. If I had to choose one then it would be ansible.windows.

Not sure if @jborean93 or @nitzmahone would agree with me though.

@jborean93
Copy link
Contributor Author

I essentially agree with @briantist here and think right now they still belong in ansible/ansible. The only reason why is that all the code to execute modules (including the stuff I added in this PR) is still contained in this repo. Using the -Optional addition as an example, it was added in Ansible 2.12 which is not related to any collection version but ansible-core itself.

I don't really mind either way where it goes but my preference would be here. If you still wish to move it out then ansible.windows would be the next best place as it's essentially the only "supported" Windows collection from us.

@samccann
Copy link
Contributor

@jborean93 @briantist cool thanks for the detailed updates. Here it stays for now!

@samccann samccann merged commit 4c738b0 into ansible:devel Dec 20, 2021
@jborean93 jborean93 deleted the win-dev-guide branch December 20, 2021 19:20
@jborean93
Copy link
Contributor Author

jborean93 commented Dec 20, 2021 •
edited

Thanks for the review @samccann!

@ansible ansible locked and limited conversation to collaborators Jan 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@briantist briantist briantist approved these changes

@samccann samccann samccann approved these changes

Assignees
No one assigned
Labels
affects_2.13 core_review In order to be merged, this PR must follow the core review workflow. docs_only All changes are to files within the docs/docsite/ directory docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jborean93 @samccann @briantist @ansibot