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

[WIP] Adding docs #5

Merged
merged 1 commit into from
May 29, 2020
Merged

Conversation

abadger
Copy link
Contributor

@abadger abadger commented May 4, 2020

Adding a script which will extract docs from collections and format as rst. This script will also be usable by third parties who are looking to build documentation for their own websites.

  • Build docs for stable
    • Parse the collections from a ansibulled deps file
    • Download ansible-base that was used in the deps file
    • Download collections at those versions
    • Parse docs out of ansible-base plugins
    • Parse docs out of collections
    • Normalize the parsed output
    • Construct rst from parsed data
    • Construct index.rst from the parsed data
    • Output to the right directory structure
  • Fix options display (full_path needs to be calculated)
  • Fix linking between modules (the anchors aren't conducive to construction for M()
  • Fix any ansibulled commands that were broken by these changes
  • Website documentation command in ansible/ansible repo
  • Switch from using ansible-doc to parse to our own code for robustness
  • Build docs for devel (Possibly later PR)
  • Build docs for a collection (Later PR)
  • Build docs for a single plugin (Later PR)
  • Write tests (Later PR)
  • Write documentation of ansibulled itself (Later PR)

@lgtm-com
Copy link

lgtm-com bot commented May 7, 2020

This pull request introduces 5 alerts when merging a3cff46 into fa46cb7 - view on LGTM.com

new alerts:

  • 2 for Use of the return value of a procedure
  • 2 for Variable defined multiple times
  • 1 for Unused local variable

@codecov-io
Copy link

codecov-io commented May 7, 2020

Codecov Report

Merging #5 into master will increase coverage by 11.48%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #5       +/-   ##
===========================================
+ Coverage    3.76%   15.25%   +11.48%     
===========================================
  Files          17        5       -12     
  Lines        1646      400     -1246     
  Branches      318       43      -275     
===========================================
- Hits           62       61        -1     
+ Misses       1580      335     -1245     
  Partials        4        4               
Impacted Files Coverage Δ
ansibulled/build_acd_commands.py 0.00% <0.00%> (ø)
ansibulled/cli/ansibulled_changelog.py
ansibulled/cli/ansibulled.py
ansibulled/changelog/config.py
ansibulled/changelog/changes.py
ansibulled/changelog/ansible.py
ansibulled/changelog/plugins.py
ansibulled/changelog/lint.py
ansibulled/changelog/rst.py
ansibulled/changelog/fragment.py
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1764b0c...baaed44. Read the comment docs.

@abadger abadger force-pushed the adding-docs branch 3 times, most recently from 8195d7e to e756821 Compare May 11, 2020 14:42
@lgtm-com
Copy link

lgtm-com bot commented May 11, 2020

This pull request introduces 9 alerts when merging e756821 into 36fcbe1 - view on LGTM.com

new alerts:

  • 4 for Unused import
  • 2 for Use of the return value of a procedure
  • 2 for Variable defined multiple times
  • 1 for Module is imported with 'import' and 'import from'

@abadger abadger force-pushed the adding-docs branch 2 times, most recently from 41c5b2d to baaed44 Compare May 14, 2020 15:56
@codecov-commenter
Copy link

codecov-commenter commented May 20, 2020

Codecov Report

Merging #5 into master will increase coverage by 11.48%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #5       +/-   ##
===========================================
+ Coverage    3.76%   15.25%   +11.48%     
===========================================
  Files          17        5       -12     
  Lines        1646      400     -1246     
  Branches      318       43      -275     
===========================================
- Hits           62       61        -1     
+ Misses       1580      335     -1245     
  Partials        4        4               
Impacted Files Coverage Δ
ansibulled/build_acd_commands.py 0.00% <0.00%> (ø)
ansibulled/changelog/config.py
ansibulled/changelog/fragment.py
ansibulled/changelog/ansible.py
ansibulled/cli/ansibulled.py
ansibulled/cli/ansibulled_lint_changelog_yaml.py
ansibulled/changelog/changes.py
ansibulled/changelog/lint.py
ansibulled/changelog/plugins.py
ansibulled/cli/ansibulled_changelog.py
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1764b0c...161e03d. Read the comment docs.

@abadger abadger force-pushed the adding-docs branch 9 times, most recently from 509d74b to d714c0c Compare May 27, 2020 01:20
except ValueError:
# Ansible doesn't give itself any names
namespace = 'ansible'
collection = 'builtins'
Copy link
Collaborator

@felixfontein felixfontein May 28, 2020

Choose a reason for hiding this comment

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

Suggested change
collection = 'builtins'
collection = 'builtin'

The synthetic collection included in ansible-base is called ansible.builtin, not ansibe.builtins. (If you grep for it, you'll find several matches for ansible.builtin, but none for ansible.builtins.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I saw this too. I've got the same simple fix queued, but I wondered whether I should modify the schemas to add ansible builtin to any DOCUMENTATION entry that only has a short name and then removing this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, after talking with nitzmahone about how the collection loader works, I don't think that the schema is the right place anymore. The reason is theoretical but I think it's valid.

Modules in ANSIBLE_LIBRARY (and same for the other plugin paths) are actually unknown to the collection loader. So to ansible, the plugins shipped with ansible-base are part of the synthetic ansible-builtin collection but the ones in ANSIBLE_LIBRARY path are known to not exist in a collection at all. The ansible-doc parser is not passing that information on to us.

If we normalized this in the schema, then we'd falsely be adding ANSIBLE_LIBRARY plugins into ansible.builtin. Since the idea for the schema is that it could be easily reused by other parties, I don't think we should perform this kludge there. I think the right place to fix up the plugin names is in the backend parser. Until we write our own, we have to rely on what ansible-doc does. what ithink that means is we should put the kludge into the ansible_doc backend. After we get the output back from ansible_doc, change the plugin names that need them to include ansible.builtins there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abadger abadger force-pushed the adding-docs branch 2 times, most recently from 3ca2744 to d1377e4 Compare May 29, 2020 04:57
@abadger
Copy link
Contributor Author

abadger commented May 29, 2020

The goal is to get this merged today so that we can have a test docsite build by next week. Towards that end, I've opened the following issues for known bugs in this PR:

I've also opened issues for all of the "TODO later" items.

Once this passes the automated tests, it will get merged.

[_] Build docs for stable
    [x] Parse the collections from a antsibull deps file
    [x] Download ansible-base that was used in the deps file
    [x] Download collections at those versions
    [x] Install the collections into the ansible-base that was downloaded
    [x] Parse docs out of ansible-base plugins
    [x] Parse docs out of collections
    [x] Construct rst from parsed data
    [x] Output to the right directory structure
    [x] Construct index.rst from the parsed data
[_] Fix options output (full_path needs to be constructed)
[_] Fix linking between modules (the anchors are not easily constructable by M()
[_] Build docs for devel
    [_] Parse the deps from an antsibull .in file
    [_] Download the latest versions of those collections
    [_] Git clone the latest version of ansible-base
[_] Build docs for a collection
    [_] ?
[_] Build docs for a single plugin
    [_] ?

Changes:

* Add type info for everything antsibull-docs uses.
* Move code to install collections to its own python module for reuse by
  antsibull-docs
* Move some constants that are shared between antsibull-changelog and
  antsibull-docs into a constants module.
* Move code for untarring tarballs to their own module for reuse with
  antsibull-docs.
* Create a module that combines venv and sh functionality.  Makes it
  easier to run programs inside of a venv.
* Change changelog.ansible.get_documentable_plugins() to return a frozenset.
* Create a module for parsing documentation out of plugins.
* Normalize the documentation via a schema.
* Add the ability tto add ref entries via a `R()` macro in
  DOCUMENTATION.
* Fix the vendored collections ImmutableDict to do equality testing
@abadger abadger merged commit 984aff1 into ansible-community:master May 29, 2020
@abadger abadger deleted the adding-docs branch May 29, 2020 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants