attempting to wrangle feedback from git comments.

please edit specific lines when possible to ensure your feedback
is incorporated properly. thanks!
This commit is contained in:
Sandra Wills 2016-04-08 12:37:49 -04:00 committed by James Cammarata
parent 640f4e79bd
commit 59456af8be

View file

@ -485,24 +485,26 @@ Module checklist
The following checklist items are important guidelines for people who want to contribute to the development of modules to Ansible on GitHub. Please read the guidelines before you submit your PR/proposal.
* The shebang should always be #!/usr/bin/python, this allows ansible_python_interpreter to work
* The shebang should always be ``#!/usr/bin/python``, this allows ansible_python_interpreter to work
* Modules must be written to support Python 2.4. If this is not possible, required minimum python version and rationale should be explained in the requirements section in DOCUMENTATION.
* Documentation: Make sure it exists
* Module documentation should briefly and accurately define what each module and option does, and how it works with others in the underlying system. Documentation should be written for broad audience--readable both by experts and non-experts. This documentation is not meant to teach a total novice, but it also should not be reserved for the Illuminati (hard balance).
* `required` should always be present, be it true or false
* If `required` is false you need to document `default`, even if the default is 'null' (which is the default if no parameter is supplied). Make sure default parameter in docs matches default parameter in code.
* `default` is not needed for `required: true`
* Remove unnecessary doc like `aliases: []` or `choices: []`
* The version is not a float number and value the current development version
* Verify that arguments in doc and module spec dict are identical
* For password / secret arguments no_log=True should be set
* Requirements should be documented, using the `requirements=[]` field
* Author should be set, name and github id at least
* Made use of U() for urls, C() for files and options, I() for params, M() for modules?
* GPL 3 License header
* Does module use check_mode? Could it be modified to use it? Document it
* Examples: make sure they are reproducible
* If an argument takes both C(True)/C(False) and C(Yes)/C(No), the documentation should use C(True) and C(False).
* Descriptions should always start with a Capital letter and end with a full stop. Consistency always helps.
* The `required` setting should always be present, be it true *or* false
* If `required` is false, you should document `default`, even if the default is 'null' (which is the default if no parameter is supplied). Make sure default parameter in docs matches default parameter in code.
* Documenting `default` is not needed for `required: true`.
* Remove unnecessary doc like `aliases: []` or `choices: []`.
* The version is not a float number and value the current development version.
* Verify that arguments in doc and module spec dict are identical.
* For password / secret arguments no_log=True should be set.
* Requirements should be documented, using the `requirements=[]` field.
* Author should be set, with their name and their github id, at the least.
* Ensure that you make use of U() for urls, C() for files and options, I() for params, M() for modules.
* If an optional parameter is sometimes required this need to be reflected in the documentation, e.g. "Required when C(state=present)."
* Verify that a GPL 3 License header is included.
* Does module use check_mode? Could it be modified to use it? Document it. Documentation is everyone's friend.
* Examples--include them whenever possible and make sure they are reproducible.
* Document the return structure of the module. Refer to :ref:`common_return_values` and :ref:`module_documenting` for additional information.
* Predictable user interface: This is a particularly important section as it is also an area where we need significant improvements.
* Name consistency across modules (weve gotten better at this, but we still have many deviations).
@ -513,18 +515,27 @@ The following checklist items are important guidelines for people who want to c
* Always return useful data, even when there is no change.
* Be consistent about returns (some modules are too random), unless it is detrimental to the state/action.
* Make returns reusable--most of the time you don't want to read it, but you do want to process it and re-purpose it.
* Return diff if in diff mode
* Return diff if in diff mode. This is not required for all modules, as it won't make sense for certain ones, but please attempt to include this when applicable).
* Code: This applies to all code in general, but often seems to be missing from modules, so please keep the following in mind as you work.
* Validate upfront--fail fast and return useful and clear error messages.
* Defensive programming--modules should be designed simply enough that this should be easy. Modules should always handle errors gracefully and avoid direct stacktraces. Ansible deals with this better in 2.0 and returns them in the results.
* Fail predictably--if we must fail, do it in a way that is the most expected. Either mimic the underlying tool or the general way the system works.
* Modules should not do the job of other modules, that is what roles are for. Less magic is more.
* Don't reinvent the wheel. Part of the problem is that code sharing is not that easy nor documented, we also need to expand our base functions to provide common patterns (retry, throttling, etc).
* Support check mode. For more information, refer to :ref:`check_mode_drift` and :ref:`check_mode_dry`.
* Support check mode. This is not required for all modules, as it won't make sense for certain ones, but please attempt to include this when applicable). For more information, refer to :ref:`check_mode_drift` and :ref:`check_mode_dry`.
* Exceptions: The module must handle them. (exceptions are bugs)
* Give out useful messages on what you were doing and you can add the exception message to that.
* Avoid catchall exceptions, they are not very useful unless the underlying API gives very good error messages pertaining the attempted action.
* The module must not use sys.exit() --> use fail_json() from the module object
* Module-dependent guidelines: Additional module guidelines may exist for certain families of modules.
* Be sure to check out the modules themselves for additional information.
* Amazon: https://github.com/ansible/ansible-modules-extras/blob/devel/cloud/amazon/GUIDELINES.md
* Modules should make use of the "extends_documentation_fragment" to ensure documentation available. For example, the AWS module should include::
extends_documentation_fragment:
- aws
- ec2
* The module must not use sys.exit() --> use fail_json() from the module object.
* Import custom packages in try/except and handled with fail_json() in main() e.g.::
try:
@ -534,7 +545,7 @@ The following checklist items are important guidelines for people who want to c
HAS_LIB=False
* The return structure should be consistent, even if NA/None are used for keys normally returned under other options.
* Are module actions idempotent? If not document in the descriptions or the notes
* Are module actions idempotent? If not document in the descriptions or the notes.
* Import module snippets `from ansible.module_utils.basic import *` at the bottom, conserves line numbers for debugging.
* Call your :func:`main` from a conditional so that it would be possible to
test them in the future example::
@ -555,6 +566,7 @@ The following checklist items are important guidelines for people who want to c
* When fetching URLs, please use either fetch_url or open_url from ansible.module_utils.urls
rather than urllib2; urllib2 does not natively verify TLS certificates and so is insecure for https.
Windows modules checklist
`````````````````````````
* Favour native powershell and .net ways of doing things over calls to COM libraries or calls to native executables which may or may not be present in all versions of windows