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

Docs - Split Developing collections page, add info on optional module_utils #74105

Merged
merged 8 commits into from Apr 23, 2021
Merged

Docs - Split Developing collections page, add info on optional module_utils #74105

merged 8 commits into from Apr 23, 2021

Conversation

acozine
Copy link
Contributor

@acozine acozine commented Apr 1, 2021

SUMMARY

The current page on Developing collections is very long. This PR splits it into a section.

It also adds a paragraph about using optional module utilities with try/except blocks so collections can support multiple versions of ansible/ansible-core.

Related to #73832 and #73423.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

docs.ansible.com
collections

@acozine acozine added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. docs This issue/PR relates to or includes documentation. docs_only All changes are to files within the docs/docsite/ directory labels Apr 1, 2021
@acozine acozine requested a review from nitzmahone April 1, 2021 22:13
@ansibot ansibot added affects_2.11 docsite This issue/PR relates to the documentation website. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. 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 Apr 1, 2021
@acozine
Copy link
Contributor Author

acozine commented Apr 5, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ansibot ansibot removed the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label Apr 5, 2021
nitzmahone
nitzmahone previously requested changes Apr 5, 2021
View reviewed changes
Copy link
Member

@nitzmahone nitzmahone left a comment

Choose a reason for hiding this comment

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

Split LGTM at a glance, as does the changed intro paragraph for the optional module_utils. One minor clarification probably needed on the module_utils stuff, but otherwise +1 from me.

docs/docsite/rst/dev_guide/developing_collections_specifics.rst Outdated
else:
module.fail_json('respawn is not available in Ansible < 2.11, ensure that foopkg is installed')

The optional module_utils behavior also applies to module imports from collections.
Copy link
Member

Choose a reason for hiding this comment

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

Reading this back, it could be incorrectly interpreted that we're allowing imports of Ansible modules from collections. Maybe reword to something like:

Suggested change
The optional module_utils behavior also applies to module imports from collections.
The optional import behavior also applies to module_utils imported from collections.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @nitzmahone for the review. I changed the line number in a different edit, so the suggestion is "outdated" and I can't commit it from here. I implemented this exact change in 4b03edc.

@ansibot
Copy link
Contributor

ansibot commented Apr 5, 2021

The test ansible-test sanity --test docs-build [explain] failed with 1 error:

docs/docsite/rst/dev_guide/developing_collections_creating.rst:52:0: undefined-label: undefined label: collections_structure (if the link has no caption the label must precede a section header)

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. and removed ci_verified Changes made in this PR are causing tests to fail. labels Apr 5, 2021
@acozine acozine changed the title WIP: Docs - Split Developing collections page, add info on optional module_utils Docs - Split Developing collections page, add info on optional module_utils Apr 5, 2021
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. core_review In order to be merged, this PR must follow the core review workflow. labels Apr 5, 2021
@acozine acozine dismissed nitzmahone’s stale review April 6, 2021 14:39

Review suggestion implemented in 4b03edc. Thanks @nitzmahone.

@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 Apr 6, 2021
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Apr 6, 2021
@ansibot ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Apr 6, 2021
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Apr 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. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Apr 20, 2021
@ansibot ansibot removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Apr 20, 2021
@acozine
Copy link
Contributor Author

acozine commented Apr 20, 2021

Rebased and ready for final review.

@ansibot
Copy link
Contributor

ansibot commented Apr 20, 2021

The test ansible-test sanity --test docs-build [explain] failed with 4 errors:

docs/docsite/rst/dev_guide/developing_collections_creating.rst:0:0: not-in-toc-tree: document isn't included in any toctree
docs/docsite/rst/dev_guide/developing_collections_distributing.rst:0:0: not-in-toc-tree: document isn't included in any toctree
docs/docsite/rst/dev_guide/developing_collections_shared.rst:0:0: not-in-toc-tree: document isn't included in any toctree
docs/docsite/rst/dev_guide/developing_collections_testing.rst:0:0: not-in-toc-tree: document isn't included in any toctree

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. labels Apr 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. and removed ci_verified Changes made in this PR are causing tests to fail. labels Apr 22, 2021
@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 Apr 22, 2021
@samccann samccann merged commit c90922e into ansible:devel Apr 23, 2021
acozine added a commit to acozine/ansible that referenced this pull request Apr 28, 2021
@acozine
Docs - Split Developing collections page, add info on optional module…
94c8f8d 
…_utils (ansible#74105)

*

(cherry picked from commit c90922e)
@acozine acozine mentioned this pull request Apr 28, 2021
Merged
acozine added a commit that referenced this pull request Apr 28, 2021
@acozine @Akasurde
@samccann @moreati @Hu6li @ernst-s @hs0210 @dhx-mike-palandra @jakelevinez @tremble @lidimayra @Alexhha @wenottingham
Backport/2.11/docs2 (#74484)
af07c35 
* Add description for COLLECTIONS_SCAN_SYS_PATH (#74351)

Fixes: #74275

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
(cherry picked from commit 567361b)

* lighten navigation background to make section labels easier to read for core docs (#74356)

* make section labels for /ansible-core/ docs easier to read, with black text and lighter gray background

(cherry picked from commit 6119fb0)

* Correct splitext() description, and example (#74377)

`splitext()` returns a 2-tuple of strings, and the last element of the return value includes the `.`

(cherry picked from commit c295de6)

* Using "~" instead of "+" for concatination (#74364)

Changed FAQ examples to conform with the Jinja documentation:
If both values on either side of a plus/+ are numbers, they will be added whereas using "~" will convert all operands into strings and then concatenate them. Closes #73799.

(cherry picked from commit e6a5245)

* Docs - Split Developing collections page, add info on optional module_utils (#74105)

*

(cherry picked from commit c90922e)

* Add weos4 network platform to documentation (#74088)

* Add weos4 network platform to documentation

* Fix small format issues

(cherry picked from commit 7ca5ded)

* Fix typo in Makefile (#74396)

Fixed minor typo specfic -> specific

(cherry picked from commit 4880fee)

* Update complex_data_manipulation.rst (#72509)

(cherry picked from commit c2985c4)

* Update VMware library installation docs (#71219)

Depending upon OS/distro, please use pip/pip3.

(cherry picked from commit ddfc648)

* Update AWS dev guides to use collections utils and fragments (#72312)

(cherry picked from commit cf08c23)

* Use is_boto3_error_code in 'standard' example (#72313)

Use is_boto3_error_code in 'standard' example rather than e.response['Error']['Code'] (#72313)

Co-authored-by: Sloane Hertel <shertel@redhat.com>
(cherry picked from commit 63afb33)

* Update Kubernetes collection name in docs (#74440)

(cherry picked from commit 8d499bb)

* Update argcomplete docs links on installation guide (#74410)

Link on installation docs is outdated. Switch to currently docs at: https://kislyuk.github.io/argcomplete/

(cherry picked from commit f97787c)

* fix spacing to fix header, reorg contributing page (#74421)

Co-authored-by: John R Barker <john@johnrbarker.com>
(cherry picked from commit 9d9b08b)

* Update first_found documentation (#70502)

* import_tasks do not work with loop, use use include_tasks instead
* update documentation

(cherry picked from commit bacede7)

* Product-related updates. (#74454)

(cherry picked from commit 34c9ed8)

Co-authored-by: Abhijeet Kasurde <akasurde@redhat.com>
Co-authored-by: Sandra McCann <samccann@redhat.com>
Co-authored-by: Alex Willmer <alex@moreati.org.uk>
Co-authored-by: Hublerho <43293510+Hublerho@users.noreply.github.com>
Co-authored-by: Ernst Oudhof <17832702+ernst-s@users.noreply.github.com>
Co-authored-by: Hu Shuai <hus.fnst@cn.fujitsu.com>
Co-authored-by: dhx-mike-palandra <45608336+dhx-mike-palandra@users.noreply.github.com>
Co-authored-by: jakelevinez <31458570+jakelevinez@users.noreply.github.com>
Co-authored-by: Mark Chappell <mchappel@redhat.com>
Co-authored-by: Lidiane Taquehara <lidi.mayra@gmail.com>
Co-authored-by: Alex Domoradov <alex.hha@gmail.com>
Co-authored-by: Bill Nottingham <notting@redhat.com>
@acozine acozine deleted the split_coll_dev_docs branch April 30, 2021 19:05
@ansible ansible locked and limited conversation to collaborators May 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@samccann samccann samccann approved these changes

@nitzmahone nitzmahone nitzmahone left review comments

Assignees
No one assigned
Labels
affects_2.11 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. pr_day Has been reviewed during a PR review Day 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
5 participants
@acozine @ansibot @nitzmahone @samccann @s-hertel