[WIP, needs vote before merging] collection_requirements.rst: update #1422
Conversation
ansible-documentation-bot
bot
added
the
sc_approval
|
|
||
| * If the collection works with Ansible 2.9, then this should be set to `>=2.9.10` | ||
| * It is usually better to avoid adding `<2.11` as a restriction, since this for example makes it impossible to use the collection with the current ansible-base devel branch (which has version 2.11.0.dev0) | ||
| * If the collection works with ansible-core 2.16, then this should be set to `>=2.16`. |
There was a problem hiding this comment.
I don't think it makes sense to change this line to reference 2.16. The point of the line was less as an example, and more to specify that if Ansible 2.9 is supported, it should not support <2.9.10 because of various collection-supporting bugfixes that were not included in earlier versions.
As a result, the reference to "Ansible" in the line above it is more accurate than changing it to ansible-core, since we're not really talking about the specific package name, but more like "Ansible" the product.
If we were to disallow collections that support 2.9 at all, then I think we would remove this line (or change it to set an absolute minimum for the value in runtime.yml) but I don't any reason why we would do that.
There was a problem hiding this comment.
@briantist could you please take a look at the current section state (we've committed several changes in it) and suggest changes if still needed?
| * If the collection works with Ansible 2.9, then this should be set to `>=2.9.10` | ||
| * It is usually better to avoid adding `<2.11` as a restriction, since this for example makes it impossible to use the collection with the current ansible-base devel branch (which has version 2.11.0.dev0) | ||
| * If the collection works with ansible-core 2.16, then this should be set to `>=2.16`. | ||
| * It is usually better to avoid adding `<2.16` as a restriction, since this for example makes it impossible to use the collection with the current ansible-core devel branch. |
There was a problem hiding this comment.
Maybe we should consider rewording this to avoid having to keep changing the versions referenced? Something along the lines of
it is usually better to avoid setting an upper-bound on the supported version...
or
it is usually better to use no upper bound or to set it to the next expected version, rather than the current stable version...
There was a problem hiding this comment.
should we maybe remove it at all? I don't remember any such cases in submitted collections. i don't think anybody will come up with this solution on its own if it's not really necessary in their case. We can always put it back if we notice. It's a recommendation anyway.
Removed not to forget (but ready to put it back if needed)
| * If you for some reason really have to specify version numbers of Ansible or of another collection, you also have to provide ``version_added_collection: collection_name``. We strongly recommend to NOT do this. | ||
| * Include ``version_added`` when you add new content (modules, plugins, options) to an existing collection. The values are shown in the documentation and can be useful, but you do not need to add ``version_added`` to every option, module, and plugin when creating a new collection. | ||
| * Include ``version_added`` when you add new content (modules, plugins, options, return values) to an existing collection. The values are shown in the documentation and can be useful, but you do not need to add ``version_added`` to every option, module, and plugin when creating a new collection. |
There was a problem hiding this comment.
Not sure how to word this but it also applies to config manager sub-sections of plugin options, like ini, vars, etc.
There was a problem hiding this comment.
good point, me too, any suggestions are welcome
There was a problem hiding this comment.
if you can point me to an example, I might be able to help with the wording. Not sure what a config-manager subsection might be (other than the two given in the comment).
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
| Existing repositories SHOULD be converted to use ``main``. | ||
|
|
There was a problem hiding this comment.
Why remove this?
@briantist this was relevant after that decision to convert was made and we converted where we could and created issues in all other repos under ansible-collections. Who wanted already did it and new repos are created there with main. Maybe we should rephrase it somehow UPDATE: yes, i see now that there's the requirement for all newly submitted repos to have main, so this line is extra imo
| @@ -420,10 +417,9 @@ CI Testing | |||
| 1. A dangerous module parameter has been deprecated or removed, and code is present to inform the user that they should not use this specific parameter anymore or that it stopped working intentionally. | |||
| 2. Module parameters are only used to pass in data from an accompanying action plugin. | |||
|
|
|||
| * All entries in ignores.txt MUST have a justification in a comment in the ignore.txt file for each entry. For example ``plugins/modules/docker_container.py use-argspec-type-path # uses colon-separated paths, can't use type=path``. | |||
| * Reviewers can block acceptance of a new collection if they don't agree with the ignores.txt entries. | |||
| * All entries in ignore-*.txt files MUST have a justification in a comment in the files for each entry. For example ``plugins/modules/docker_container.py use-argspec-type-path # uses colon-separated paths, can't use type=path``. | |||
There was a problem hiding this comment.
| * All entries in ignore-*.txt files MUST have a justification in a comment in the files for each entry. For example ``plugins/modules/docker_container.py use-argspec-type-path # uses colon-separated paths, can't use type=path``. | |
| * All entries in `ignore-*.txt` files MUST have a justification in a comment in the files for each entry. For example `plugins/modules/docker_container.py use-argspec-type-path # uses colon-separated paths, can't use type=path`. |
There was a problem hiding this comment.
I'm not an expert but i think two apostrophes would work better here
| * :ref:`Ansible documentation format <module_documenting>` and the :ref:`style guide <style-guide>`. | ||
| * To pass the Ansible :ref:`sanity tests <testing-sanity>`. | ||
| * To have :ref:`unit <unit-tests>`_and / or :ref:`integration tests <integration-tests>` according to the corresponding sections of this document. | ||
| To be included in the `ansible` package, collections MUST meet the criteria marked with ``MUST`` in this document. |
There was a problem hiding this comment.
| To be included in the `ansible` package, collections MUST meet the criteria marked with ``MUST`` in this document. | |
| To be included in the `ansible` package, collections MUST meet the criteria marked with `MUST` in this document. |
There was a problem hiding this comment.
i think this is a correct formatting (am not an expert though)
There was a problem hiding this comment.
Looks good @Andersson007 I've read through this a couple of times to try and come up with comments other than grammar nits but it seems fine. It looks a bit more straightforward now. I'll wait and see how the vote goes before approving in case there are additional changes.
| @@ -126,7 +114,9 @@ Standards for developing module and plugin utilities | |||
| ==================================================== | |||
|
|
|||
| * ``module_utils`` and ``plugin_utils`` can be marked for only internal use in the collection, but they MUST document this and MUST use a leading underscore for file names. | |||
| * It is a breaking change when you make an existing ``module_utils`` private and in that case the collection requires a major version bump. | |||
|
|
|||
| * It is a breaking change when you make an existing ``module_utils`` private and in that case the collection requires a major version bump. | |||
There was a problem hiding this comment.
| * It is a breaking change when you make an existing ``module_utils`` private and in that case the collection requires a major version bump. | |
| * It is a breaking change when you make an existing ``module_utils`` private, in which case the collection requires a major version bump. |
There was a problem hiding this comment.
Should this be formatted as a note rather than an indented list item?
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
https://github.com/ansible/ansible-documentation/pull/1422/files#diff-17918ba4d47cb68e11b669412c5ea11b87bd8b5944b04720c6b74d104c692829R253 (Not sure that link will work, but the galaxy docs links under Naming are no longer correct and should be updated.
| @@ -189,7 +176,7 @@ Modules & Plugins | |||
| Other directories | |||
| ----------------- | |||
|
|
|||
| Collections MUST not use files outside ``meta/``, ``plugins/``, ``roles/`` and ``playbooks/`` in any plugin, role, or playbook that can be called by FQCN, used from other collections, or used from user playbooks and roles. A collection must work if every file or directory is deleted from the installed collection except those four directories and their contents. | |||
| Collections MUST not use files outside ``meta/``, ``plugins/``, ``roles/`` and ``playbooks/`` in any plugin, role, or playbook that can be called by FQCN, used from other collections, or used from user playbooks and roles. | |||
There was a problem hiding this comment.
how do we handle collections that include eda plugins? I know docs, ansible-doc etc can't work with them, but they are in an extensions/eda dir today. Would that mean the collection is excluded from the package?
| * If you for some reason really have to specify version numbers of Ansible or of another collection, you also have to provide ``version_added_collection: collection_name``. We strongly recommend to NOT do this. | ||
| * Include ``version_added`` when you add new content (modules, plugins, options) to an existing collection. The values are shown in the documentation and can be useful, but you do not need to add ``version_added`` to every option, module, and plugin when creating a new collection. | ||
| * Include ``version_added`` when you add new content (modules, plugins, options, return values) to an existing collection. The values are shown in the documentation and can be useful, but you do not need to add ``version_added`` to every option, module, and plugin when creating a new collection. |
There was a problem hiding this comment.
if you can point me to an example, I might be able to help with the wording. Not sure what a config-manager subsection might be (other than the two given in the comment).
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
| @@ -239,7 +226,7 @@ Note that the porting guide is compiled from ``changelogs/changelog.yaml`` (sect | |||
| Versioning and deprecation | |||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~ | |||
|
|
|||
| * Collections MUST adhere to `semantic versioning <https://semver.org/>`_. | |||
| * Collections MUST adhere to `Semantic versioning convention <https://semver.org/>`_. | |||
| * To preserve backward compatibility for users, every Ansible minor version series (x.Y.z) will keep the major version of a collection constant. If Ansible 3.0.0 includes ``community.general`` 2.2.0, then each 3.Y.z (3.1.z, 3.2.z, and so on) release will include the latest ``community.general`` 2.y.z release available at build time. Ansible 3.y.z will **never** include a ``community.general`` 3.y.z release, even if it is available. Major collection version changes will be included in the next Ansible major release (4.0.0 in this example). | |||
There was a problem hiding this comment.
All of these bullet points below are clarifying semantic versioning. Maybe it will read better if they are all indented further so they look like a subset of semantic versioning.
|
Another general comment. The top sections are clear on MUST and SHOULD but as it gets later in the page, there is a lot of contextual information that tends to clutter the MUST etc. So maybe each section should start with the MUST sentences, and then under each, indent, and add the context. So for example, under python compatibility , it could have controller environment and other environment subheadings with their MUST statements. Then under the MUST, indent and add the contextual details of what each python environment means. The goal would be that a knowledgable collection maintainer can more easily skim the document to verify they meet all the MUST statements. But a newer collection creator still has the details they need to understand each MUST statement. |
it's still WIP, the call was about bringing up thoughts of what else to kick out/simplify, just a fyi, thanks for the feedback! |
@samccann something like #1459 ? (not sure if it's a good idea to create a separate PR for that though..). Folks please take a look |
|
@Andersson007 yes, that other PR makes that section easier to read! |
Andersson007
force-pushed
the
update_inclusion_reqs
branch
from
53bc981
to
f5ca31c
Compare
| * The collection's CoC MUST be compatible with the :ref:`code_of_conduct`. | ||
| * The collections SHOULD consider using the Ansible CoC if they do not have a CoC that they consider better. | ||
| * The :ref:`Diversity and Inclusion working group <working_group_list>` may evaluate all CoCs and object to a collection's inclusion based on the CoCs contents. | ||
| * The CoC MUST be linked from the ``README.md`` file, or MUST be present or linked from the ``CODE_OF_CONDUCT.md`` file in the collection root. |
There was a problem hiding this comment.
Why not having it be linked from README anymore?
There was a problem hiding this comment.
Lost during re-formulation process:) back now, thanks!
There was a problem hiding this comment.
I'm also not sure if we need to have diversity & inclusion wg involvement here.. is it alive? could we just request a link to the Ansible CoC? It's not bad:) Also we don't have a mechanism to implement this and also their conclusion s probably don't have any legal power and they are not lawyers to have skills to say for sure if two documents are compatible or not.
This statement imo unreasonably complicates the process. I'll remove it now. UPDATE: i don't know how to handle this considering my point above...
Feedback is welcome
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
docs/docsite/rst/community/collection_contributors/collection_requirements.rst
Outdated
Show resolved
Hide resolved
|
|
||
| #. Use antsibull-changelog (preferred). | ||
| #. Use `antsibull-changelog <https://github.com/ansible-community/antsibull-changelog>`_: to give a consistent feel for changelogs across collections included in the ``ansible`` package. This is the **recommended** tool to maintain and generate your changelog. | ||
| #. Provide ``changelogs/changelog.yaml`` in the `correct format <https://github.com/ansible-community/antsibull-changelog/blob/main/docs/changelog.yaml-format.md>`_. (You can use ``antsibull-lint changelog-yaml /path/to/changelog.yaml`` to validate the format.) | ||
| #. Provide a link to the changelog file (self-hosted) (not recommended). |
There was a problem hiding this comment.
Should we still allow collections to use something else than changelogs/changelog.yaml? I really like the unified changelog.
If someone doesn't want to create changelog fragments, they can always use antsichaut or create something similar for their use-case.
There was a problem hiding this comment.
The unified changelog SGTM, please suggest wording
| @@ -420,10 +406,9 @@ CI Testing | |||
| 1. A dangerous module parameter has been deprecated or removed, and code is present to inform the user that they should not use this specific parameter anymore or that it stopped working intentionally. | |||
| 2. Module parameters are only used to pass in data from an accompanying action plugin. | |||
|
|
|||
| * All entries in ignores.txt MUST have a justification in a comment in the ignore.txt file for each entry. For example ``plugins/modules/docker_container.py use-argspec-type-path # uses colon-separated paths, can't use type=path``. | |||
| * Reviewers can block acceptance of a new collection if they don't agree with the ignores.txt entries. | |||
| * All entries in ``ignore-*.txt`` files MUST have a justification in a comment in the files for each entry. For example ``plugins/modules/docker_container.py use-argspec-type-path # uses colon-separated paths, can't use type=path``. | |||
There was a problem hiding this comment.
Why remove the second bullet point?
There was a problem hiding this comment.
Reviewers can do it anyway: not to put a check in the checkbox, not to put +1 for inclusion, raise objection within the default collection inclusion period - plenty of mechanisms:)
So it's an extra statement imo
…requirements.rst Co-authored-by: Brian Scholer <1260690+briantist@users.noreply.github.com>
…requirements.rst Co-authored-by: Sandra McCann <samccann@redhat.com>
…requirements.rst Co-authored-by: Sandra McCann <samccann@redhat.com>
…requirements.rst Co-authored-by: Don Naro <dnaro@redhat.com>
…requirements.rst Co-authored-by: Don Naro <dnaro@redhat.com>
…requirements.rst Co-authored-by: Felix Fontein <felix@fontein.de>
Andersson007
force-pushed
the
update_inclusion_reqs
branch
from
feddb23
to
21af233
Compare
It's time to revise and update the requirements if needed. Forum topic.
Goals:
Any suggestions are appreciated (in not touched parts as well)
Post tasks: