New collection: Scaleway

[quantumsheep]

Hi, I'm from the Scaleway's developer tools team.

We are making a new Ansible collection for our services and we wanted to know if our collection applies to be integrated into ansible-collections.

Ansible Galaxy: https://galaxy.ansible.com/scaleway/scaleway
GitHub: https://github.com/scaleway/ansible
Issues Tracker: https://github.com/scaleway/ansible/issues
GitHub Handles: @scaleway
Is the collection part of Automation Hub: No
We meet Collection Requirements: Yes

[Andersson007]

@quantumsheep hello, thanks for the submission!
As soon as there are volunteers from the community or steering committee, they'll take a look

[Andersson007]

@quantumsheep I was gonna publish the call for reviews in Bullhorn but discovered that the links to your repo, etc. are broken here and on the Galaxy page.

Also i don't think the readme (which i can see on Galaxy) contains all required information, could you please recheck.
As an example of good readme, see https://github.com/ansible-collections/community.docker and https://github.com/ansible-collections/community.mysql
Please let us know when stuff is fixed.
Thanks

[quantumsheep]

Sorry the repository was private. You should be able to access it now with extended documentation.

[quantumsheep]

Hey 👋 Any news on this ? 👀

[Andersson007]

@quantumsheep hi, I'll publish the inclusion request in Bullhorn this week. The collection will get reviewed as soon as there are volunteers.

[Andersson007]

btw are you already subscribed to bullhorn and the news-for-maintainers repo? :) Also we have many channels on Matrix. See the communication guide for details

[quantumsheep]

Thanks! I didn't knew about the news-for-maintainers repository

[Andersson007]

yep, it announces really important CI updates.
also please take a look at the maintainer guidelines: i think you''ll find a lot of useful info there

[sean-m-sullivan]

Taking a crack at this. I went through it when we were submitting and double checking ours as we found needed changes.

Public availability and communication:

• published on Ansible Galaxy with version 1.0.0 or later
• has a policy of releasing, versioning and deprecation announced to contributors and users in some way
  Has a changelog but it is not populated so hard to tell.
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
• have a publicly available issue tracker that does not require a paid level of service to create an account or view issues
• have a public git repository, releases of the collection are tagged in this repository
  Releases have not been tagged

Standards and documentation:

• adheres to semantic versioning
  Galaxy Publish's have adheared but not in the git.
• follows licensing rules
• follows the Ansible documentation standards and the style guide
• follows development conventions; as well as these other requirements:
  • modules satisfy the concept of idempotency
  • modules that only gather information are named <something>_info
  • modules that return ansible_facts are named <something>_facts and do not return non-facts
  • other modules must not allow querying information using specific state option values, or similar mechanisms (like state=get or state=query). These features should be moved to <something>_info or <something>_fact modules.
  • check_mode is supported in all _info and _facts modules
• supports Python 2.6 or greater and Python 3.5 or greater. If it does not, read the full guidelines to see if you qualify for an exception and document the unsupported Python versions in the collection README.md and in every module and plugin (or in doc fragments)
• only uses the allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return sections use FQCNs for the M(..) format macros when referring to modules
• modules (or plugins) from ansible-core use ansible.builtin. as a FQCN prefix
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added
• FQCNs are used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-core
• public plugins, roles and playbooks do not use files outside of meta/, plugins/, roles/, and playbooks/

Collection management:

• galaxy.yml having tags field set
• collection dependencies must have a lower bound on the version which is at least 1.0.0, and are all part of the ansible package
• meta/runtime.yml defines the minimal version of Ansible which the collection works with
• has changelog, preferably with changelogs/changelog.yaml\
  Empty Changelog, but has 3 versions released
• collection repository should not contain any large objects (binaries) comparatively to the current Galaxy tarball size limit of 20 MB. For example, package installers for testing purposes shouldn't be added.
• collection repository should not contain any unnecessary files like, for example, temporary files. Temporary files should be added to .gitignore.
  Recomended change: community-scaleway-1.0.0.tar.gz file in the base dir, should be added to .gitignore as *.tar.gz

Tests:

• In most cases, CI is set up via GitHub Actions.
• Check .yml files in the .github/workflows directory. There must be at least the sanity section under jobs containing the ansible-test sanity command running against all supported ansible-core versions that must be also listed there, for example, - stable-2.11, - stable-2.12, - stable-2.13.
• Check workflow runs by clicking the Actions tab in the repository's page - you're interested in Scheduled runs, runs against release commits and runs against pull requests.
• If there are no workflows in the Actions tab, ask the collection maintainers how CI is implemented.

• passed ansible-test sanity
  Must change: Lastest Sanity tests are failing
• if test/sanity/ignore*.txt exists, it MUST not contain error codes listed here
• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports
• all CI tests MUST run against every pull request
• all CI tests MUST run regularly (nightly, or at least once per week)
• sanity tests MUST run against a commit that releases the collection; if they don't pass, the collection won't be released
  No workflow to verify this one

List of Changes
Must change:

• Sanity tests failing.

Should change:

• This is my first review so not sure about how to handle this
• Empty Changelog, but has 3 versions released, and no tags in the git repository for past versions.
• Know this is a Fix in the future, but how do we handle on review.

Recomended change:

• community-scaleway-1.0.0.tar.gz file in the base dir, should be added to .gitignore as *.tar.gz

[Andersson007]

@quantumsheep cool, thanks:)
you could also review one of @sean-m-sullivan 's collection inclusion requests:)
I'm sure you'll learn a lot and your own collection will get better even better and will be handled with higher priority;)

[Andersson007]

@quantumsheep fyi we've suspended reviewing the infra collections so no need to review them, thanks.
once you have @sean-m-sullivan 's feedback incorporated, let me know, i'll try to find time to review your collection myself

[quantumsheep]

@Andersson007 @sean-m-sullivan Hey! Sorry for the delay. This is now done 😄 The sanity and integration tests are passing :)

I'm awaiting your review.

[sean-m-sullivan]

@Andersson007 I've reviewed their changes, It looks good for the next step now!

[quantumsheep]

@Andersson007 Hello 👋 Sorry to bother you, any news for the review?

[Andersson007]

Ansible Collections Checklist (short version)

For details about the following points, refer to the Collection Requirements.

Every comment should say whether the reviewer expects it to be addressed, or whether it's optional.

Note for reviewers: If you don't know how to check any of the points below, please ask maintainers of the collection you're reviewing or a Steering Committee member for clarifications in comments of corresponding inclusion discussion.

Public availability and communication:

• published on Ansible Galaxy with version 1.0.0 or later
  MUST FIX: on Galaxy the quantumsheep namespace is used, so there's also a mismatch between FQCN (scaleway.scaleway) and the Galaxy namespace. Should FQCN reflect the correct namespace, i.e. be quantumsheep.scaleway?
  UPDATE: I see now that there's https://galaxy.ansible.com/scaleway/scaleway.. So the link to Galaxy page seems irrelevant, please make it consistent.
• has a policy of releasing, versioning and deprecation announced to contributors and users in some way
  MUST FIX i don't see this. It can be a section in README like in community.crypto
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
  MUST FIX add a link to it to README I see the link
• have a publicly available issue tracker that does not require a paid level of service to create an account or view issues
• have a public git repository, releases of the collection are tagged in this repository
  MUST FIX there's a mismatch between tags in the repo and the releases on galaxy (in particular, 2.2.0 is not on Galaxy)
  UPDATE: I see it in scaleway.scaleway, please update the discussion description to point to the right galaxy page

Standards and documentation:

• adheres to semantic versioning
  MUST FIX No info that you follow SemVer in the REAMDE. You can incorporate this with the Releasing section mentioned above
• follows licensing rules
• follows the Ansible documentation standards and the style guide
  MUST FIX: On Galaxy I see FQCN quantumsheep.scaleway but in modules and in the latest README in the repo scaleway.scaleway
  MUST FIX: In the code, check_mode declared as supported but is not mentioned in the doc (in notes: or attributes: (using the latter is preferrable)
  MUST FIX: I don't think the doc satisfies the standards, see description:s, for example. Please re-check.
• follows development conventions; as well as these other requirements:
  • modules satisfy the concept of idempotency
  • modules that only gather information are named <something>_info
  • modules that return ansible_facts are named <something>_facts and do not return non-facts
  • other modules must not allow querying information using specific state option values, or similar mechanisms (like state=get or state=query). These features should be moved to <something>_info or <something>_fact modules.
  • check_mode is supported in all _info and _facts modules
• supports Python 2.6 or greater and Python 3.5 or greater. If it does not, read the full guidelines to see if you qualify for an exception and document the unsupported Python versions in the collection README.md and in every module and plugin (or in doc fragments)
  MUST FIX Sanity tests fail on Python 2.7 and 3.5, looks like yet unsupported syntax is used
  MUST FIX I see tests run against 3.8 and 3.9 only. Please make all Python related stuff consistent (README, requirements, etc.). Also the collection MUST support some versions (exceptions are only possible if dependency libs don't support them) and it must be tested against all Python versions it supports
• only uses the allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return sections use FQCNs for the M(..) format macros when referring to modules
• modules (or plugins) from ansible-core use ansible.builtin. as a FQCN prefix
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added (except cases when they were added in the very first release of the collection)
• FQCNs are used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-core
• public plugins, roles and playbooks do not use files outside of meta/, plugins/, roles/, and playbooks/

Collection management:

• galaxy.yml having tags field set
• collection dependencies must have a lower bound on the version which is at least 1.0.0, and are all part of the ansible package
• meta/runtime.yml defines the minimal version of Ansible which the collection works with
• has changelog, preferably with changelogs/changelog.yaml
  MUST FIX: there are no entries for some releases
• collection repository should not contain any large objects (binaries) comparatively to the current Galaxy tarball size limit of 20 MB. For example, package installers for testing purposes shouldn't be added.
• collection repository should not contain any unnecessary files like, for example, temporary files. Temporary files should be added to .gitignore.

Tests:

Note for reviewers: If you don't know how to check the points below, please ask maintainers of the collection you're reviewing how you can do it.

• In most cases, CI is set up via GitHub Actions.
• Check .yml files in the .github/workflows directory. There must be at least the sanity section under jobs containing the ansible-test sanity command running against all supported ansible-core versions that must be also listed there, for example, - stable-2.11, - stable-2.12, - stable-2.13.
• Check workflow runs by clicking the Actions tab in the repository's page - you're interested in Scheduled runs, runs against release commits and runs against pull requests.
• If there are no workflows in the Actions tab, ask the collection maintainers how CI is implemented.

• passed ansible-test sanity
  MUST FIX: a lot of invalid syntax errors (i tried against stable-2.15 and devel)
• if test/sanity/ignore*.txt exists, it MUST not contain error codes listed in the list of errors that must not be ignored
• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports
• all CI tests MUST run against every pull request
• all CI tests MUST run regularly (nightly, or at least once per week)
  MUST FIX: i see it's commented in the WF file
• sanity tests MUST run against a commit that releases the collection; if they don't pass, the collection won't be released

[Andersson007]

i also see that you use quantumsheep namespace on Galaxy, so there's also a mismatch between FQCN and the Galaxy namespace. I'll add a note to my review. FYI
UPDATE: i see now that there a wrong Galaxy link in the description, could you please update it? It also probably makes sense to drop that first page (quantumsheep) not to confuse users.

[quantumsheep]

Hey, thanks for your review, I will fix the issues!

We transferred the repository into our official organization, the links are now fixed.

[Andersson007]

cool, thanks, let us know when it's ready for another review

[quantumsheep]

Hey !
I updated the links (quantumsheep.scaleway -> scaleway.scaleway).

About the supported Python versions, we only support >= 3.7 (our library uses 3.7 syntax).

MUST FIX: In the code, check_mode declared as supported but is not mentioned in the doc (in notes: or attributes: (using the latter is preferrable)

Is this documented anywhere? I don't find it

[Andersson007]

@quantumsheep

Is this documented anywhere? I don't find it

ah, it's still a relatively new feature, so nope, sorry:) Please take a look at #59 (reply in thread)