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

Documentation rendering default value lists #43901

Closed
LindsayHill opened this issue Aug 9, 2018 · 6 comments · Fixed by #56041
Closed

Documentation rendering default value lists #43901

LindsayHill opened this issue Aug 9, 2018 · 6 comments · Fixed by #56041
Assignees
Labels
affects_2.7 This issue/PR affects Ansible v2.7 docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. networking Network category support:community This issue/PR relates to code supported by the Ansible community.

Comments

@LindsayHill
Copy link
Contributor

SUMMARY

Lists in docstrings are rendered in HTML and ansible-doc with Python Unicode-notation, e.g. [u'!config']. I believe it is more human-readable to render it as ['!config']

ISSUE TYPE
  • Documentation Report
COMPONENT NAME

dellos10_facts

ANSIBLE VERSION
ansible 2.7.0.dev0 (devel cdb5140ade) last updated 2018/08/09 11:30:09 (GMT -700)
  config file = /Users/lhill/github/ansible-extreme/slxos/playbooks/ansible.cfg
  configured module search path = [u'/Users/lhill/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/lhill/github/ansible/lib/ansible
  executable location = /Users/lhill/github/ansible/bin/ansible
  python version = 2.7.10 (default, Oct  6 2017, 22:29:07) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.31)]
CONFIGURATION

Not relevant - this appears in public docs site, e.g. https://docs.ansible.com/ansible/devel/modules/dellos10_facts_module.html#dellos10-facts-module

OS / ENVIRONMENT

N/A

STEPS TO REPRODUCE

NB: This applies to multiple modules, I am just using dellos10_facts as an example here.

1/ Create docstring section in Ansible module, containing a default value that is a list. For example https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/network/dellos10/dellos10_facts.py#L38
The gather_subset parameter has default value: default: [ '!config' ]

2/ View rendered HTML documentation or output of ansible-doc dellos10_facts.

Look at the gather_subset section of the HTML or ansible-doc output.

EXPECTED RESULTS

I expected the results to contain a [!config] line for the default value for gather_subset, e.g.:

- gather_subset
        When supplied, this argument will restrict the facts collected to a given subset.  Possible values for this argument
        include all, hardware, config, and interfaces.  Can specify a list of values to include a larger subset.  Values can
        also be used with an initial `[!]' to specify that a specific subset should not be collected.
        [Default: [u'!config']]

I expected the HTML output to contain ['!config']

ACTUAL RESULTS

Note it looks like this:

(venv)lindsaysmacbook:playbooks lhill$ ansible-doc dellos10_facts
> DELLOS10_FACTS    (/Users/lhill/github/ansible/lib/ansible/modules/network/dellos10/dellos10_facts.py)

        Collects a base set of device facts from a remote device that is running OS10.  This module prepends all of the base
        network fact keys with `ansible_net_<fact>'.  The facts module will always collect a base set of facts from the device
        and can enable or disable collection of additional facts.

OPTIONS (= is mandatory):

- gather_subset
        When supplied, this argument will restrict the facts collected to a given subset.  Possible values for this argument
        include all, hardware, config, and interfaces.  Can specify a list of values to include a larger subset.  Values can
        also be used with an initial `[!]' to specify that a specific subset should not be collected.
        [Default: [u'!config']]

Or in the HTML output:

image

@ansibot
Copy link
Contributor

ansibot commented Aug 9, 2018

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Aug 9, 2018

@ansibot
Copy link
Contributor

ansibot commented Aug 9, 2018

Hi @LindsayHill,

Thank you for the issue, just so you are aware we have a dedicated Working Group for network.
You can find other people interested in this in #ansible-network on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. networking Network category support:community This issue/PR relates to code supported by the Ansible community. labels Aug 9, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Aug 13, 2018
@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed support:community This issue/PR relates to code supported by the Ansible community. labels Sep 17, 2018
@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Oct 3, 2018
@acozine
Copy link
Contributor

acozine commented May 6, 2019

@acozine acozine self-assigned this May 6, 2019
@LindsayHill
Copy link
Contributor Author

I thought you might have been onto something there - but check out this cdd21e2#diff-34b216cf1a4209713450c4ef05a6a77a - the reason it looks OK in 2.5, but not 2.6 is because the underlying module had some cleanups, replacing the default: '!config' with default: [ '!config' ]

@acozine
Copy link
Contributor

acozine commented May 6, 2019

@LindsayHill oh, fascinating. So it "looks" correct but only because the underlying data was different, not because we were rendering it differently. We have done a lot of cleanup on the defaults in the module docs, but the look could definitely be improved. I've added this to the agenda for tomorrow's docs working group meeting - see ansible/community#389 (comment). Thanks for the PR - if we reach consensus that showing the [ is helpful, it's ready to merge.

@ansible ansible locked and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. networking Network category support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants