ansible / validated-content-discussion

A place for review of validated content candidates and general discussion around validated content.
GNU General Public License v3.0
2 stars 4 forks source link

candidate: infra.lvm_snapshots #20

Closed swapdisk closed 7 months ago

swapdisk commented 9 months ago

Candidate Name

infra.lvm_snapshots

Link to Github Repo

link

Review Due By

Jan 10

CHECKLIST

Business Checks:

Content Viability

Content Duplication

Technical Checks:

Content Conformity

Content Compliance

Content Testing/Validation

alisonlhart commented 8 months ago

@arnav3000 @sean-m-sullivan @djdanielsson If you have time to look over this candidate, that would be great!

ericzolf commented 8 months ago

After a quick check:

  1. not all points have been ticked, what is exactly the status?
  2. I find the idea to name the collection like one of the roles questionable, perhaps a better name is possible, covering all aspects of the collection?
  3. the variables haven't been named according to standard, with proper prefix
  4. the tasks name are not prefixed with the name of the task file
  5. the lvm_snapshots role is IMHO too complex and should be split into one role for each action to keep it/them nice and tidy (and avoid too many skipped tasks)
alisonlhart commented 8 months ago

A Requirements section with the required version of ansible should be added to the README, and the dependency of community.general should also be noted in that README section. The meta/runtime.yml "requires_ansible" key should be bumped up to 2.14.0, since all previous versions of Ansible are EoL.

swapdisk commented 7 months ago

@ericzolf and @alisonlhart, we've made changes to the candidate collection to address your kind feedback. Please take a fresh look at your convenience.

alisonlhart commented 7 months ago

@swapdisk I believe all points have been addressed and items are checked, but I'll wait for sign-off from @ericzolf and also run the changes by the ACoP before final approval.

alisonlhart commented 7 months ago

Final sign-off for this will be Jan 10.

alisonlhart commented 7 months ago

Candidate has been approved! At this time, please reach out to @ansible-pe in the #ansible-partners slack channel for assistance on next steps, or email ansiblepartners@redhat.com so we can set up your access to the infra namespace.