-
Notifications
You must be signed in to change notification settings - Fork 23.9k
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
Enable documentation in plugins #22796
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Consist used of capital letters and full stops on the descriptions would be nice to make everything consistent.
description: | ||
- a set of lists | ||
required: True | ||
EXAMPLES: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multi line yaml with key: value
Is the indentation correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how not so? just needs to be consistent, it can be 0,1... N spaces, i use 4
lib/ansible/plugins/lookup/etcd.py
Outdated
DOCUMENTATION: | ||
author: | ||
- Jan-Piet Mens (@jpmens) | ||
lookup: etc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
etcd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will fix, concentrating more on 'method' than actual docs right now
lib/ansible/utils/plugin_docs.py
Outdated
|
||
# modules that are ok that they do not have documentation strings | ||
BLACKLIST_MODULES = frozenset(( | ||
'async_wrapper', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we docs for this, or is this one of those fake modules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its 'fake module' but we can add docs, we do intend to make it 'full module'
lib/ansible/utils/plugin_docs.py
Outdated
'doc': None, | ||
'plainexamples': None, | ||
'returndocs': None, | ||
'metadata': {'metadata_version': '1.0', 'status': ['preview'], 'supported_by': 'community'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need default metadata? I wonder if in the future we can use CI to ensure this is present in all files, not just modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we plan to have 'community' plugins that are not core, so yes
consider taht most docs in the plugins are 'example' they are not full/correct, but a starting point |
9a680b9
to
3c5a107
Compare
lib/ansible/cli/doc.py
Outdated
|
||
bkey = plugin_type.upper() | ||
if bkey in plugin_docs.BLACKLIST and plugin in plugin_docs.BLACKLIST[bkey]: | ||
self.args = self.args - plugin_docs.BLACKLIST[bkey] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably what you want to do is move the call to sorted(self.args) from line 111 to line 116.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
lib/ansible/cli/doc.py
Outdated
self.args = sorted(set(self.plugin_list)) | ||
|
||
bkey = plugin_type.upper() | ||
if bkey in plugin_docs.BLACKLIST and plugin in plugin_docs.BLACKLIST[bkey]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simpler to write: if plugin in plugin_docs.BLACKLIST.get(bkey, frozenset()):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Just saw that this pattern happens in other places too.. ou can simplify everywhere that it occurs).
lib/ansible/cli/doc.py
Outdated
|
||
bkey = plugin_type.upper() | ||
if plugin in plugin_docs.BLACKLIST.get(bkey, ()): | ||
self.args = sorted(self.args) - plugin_docs.BLACKLIST[bkey] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually... this is what I meant:
self.find_plugins(path)
self.args = set(self.plugin_list)
bkey = plugin_type.upper()
if plugin in plugin_docs.BLACKLIST.get(bkey, frozenset()):
self.args = self.args - plugin_docs.BLACKLIST[bkey]
self.args = sorted(self.args)
lib/ansible/cli/doc.py
Outdated
self.find_plugins(path) | ||
|
||
bkey = plugin_type.upper() | ||
if plugin in plugin_docs.BLACKLIST.get(bkey, ()): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think plugin is defined at this point in the method.
0c9f563
to
c722f21
Compare
The test
|
Made ansible-doc more plugin agnostic We can have docs in lookup, callback, connectionm strategy, etc Use first docstring and make pepizis happy generalized module_docs to plugin_docs documented cartesian, ssh, default, jsonfile, etc as examples changed lack of docs to warning when listing made smarter about bad docstrings better blacklisting added handling of options/config/envs/etc move blacklist to find_plugins, only need once
The test
|
SUMMARY
Made ansible-doc more plugin agnostic
We can have docs in lookup, callback, connectionm strategy, etc
Use first docstring and make pepizis happy
generalized module_docs to plugin_docs
documented cartesian, ssh, default, jsonfile, etc as examples
fixes #12089
ISSUE TYPE
COMPONENT NAME
plugins
ANSIBLE VERSION
ADDITIONAL INFORMATION
TODO: