Include dellemc.enterprise_sonic collection #14
Replies: 9 comments 32 replies
-
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. Public availability and communication:
Standards and documentation:
Collection management:
Tests:
|
Beta Was this translation helpful? Give feedback.
-
|
@gundalow should we fill the checklist? |
Beta Was this translation helpful? Give feedback.
-
|
This list will be filled in by the reviewer, but you can use it to make sure in advance that your collection is in a good shape prior to the review. |
Beta Was this translation helpful? Give feedback.
-
|
@javeedf thanks for the collection and submission! I'll create a checklist later as well. It would be inconvenient to put the following output there, so i'll do it in a separate comment. If you
It must be fixed ALSO:
[FIXED] Typos: You could use my script to find some of the listed issues (for spelling use |
Beta Was this translation helpful? Give feedback.
-
|
@javeedf there's no FQCNs in a couple of EXAMPLES in /home/aklychko/ansible_collections/dellemc/enterprise_sonic/plugins/modules/sonic_aaa.py |
Beta Was this translation helpful? Give feedback.
-
|
also for most the modules: The doc requires this to be standardized. If all the modules support check_mode, you could put a line |
Beta Was this translation helpful? Give feedback.
-
|
Completed both comments |
Beta Was this translation helpful? Give feedback.
-
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. Public availability and communication:
Standards and documentation:
Collection management:
Tests:
|
Beta Was this translation helpful? Give feedback.
-
|
Thanks, @Andersson007. we will complete you feedback and get back Please clarify the below points |
Beta Was this translation helpful? Give feedback.
-
|
@javeedf YW! |
Beta Was this translation helpful? Give feedback.
-
|
@javeedf You can also include this information in the readme file itself (for example, like https://github.com/ansible-collections/community.sops#releasing-versioning-and-deprecation) or have it available on the documentation site if you have it (https://sensu.github.io/sensu-go-ansible/release_policy.html). |
Beta Was this translation helpful? Give feedback.
-
Seems to be still relevant |
Beta Was this translation helpful? Give feedback.
-
|
I'll take a closer look once the issues @Andersson007 mentioned are addressed, but here are a first few comments and questions:
|
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
|
@javeedf thanks for the quick feedback! I was mainly asking about tests/regression since ansible-test does not use/know that directory; it would use tests/integration/targets instead. The content of tests/regression/roles looks very closely to what I would expect in tests/integration/targets, so I'm wondering why you don't use |
Beta Was this translation helpful? Give feedback.
-
|
@felixfontein is moving the "tests/regression" script into "tests/integration/targets", a blocker for collection inclusion process? we would like to take this change later sometime |
Beta Was this translation helpful? Give feedback.
-
No, this is a nice-to-have thing. Feel free to move them at a later time. |
Beta Was this translation helpful? Give feedback.
-
|
@javeedf What about p. 1 which I also spotted in my review checklist?
|
Beta Was this translation helpful? Give feedback.
-
|
Just a reminder: collections that want to be part of the Ansible 4.0.0 package need to get approval by 2021-04-26 (full schedule). This inclusion request still has some open issues that need to get addressed by then. |
Beta Was this translation helpful? Give feedback.
-
|
@tadeboro we are working on this |
Beta Was this translation helpful? Give feedback.
-
|
@Andersson007 @felixfontein @tadeboro addressed all your comments. Please let us know yoir feedback. Regarding the tag, after the completion of all your comments. I will upload to galaxy with release 1.0.3 |
Beta Was this translation helpful? Give feedback.
-
|
@Andersson007 we are done with our changes. please let us know your feedback |
Beta Was this translation helpful? Give feedback.
-
|
Last reminder for inclusion submission Collections that want to be part of the Ansible 4.0.0 package need to get approval by 2021-04-26, which is just one week away (full schedule). Make sure you submit your collection for the final review a few days before the deadline (reviews take a non-trivial amount of time) and that your collection does meet all criteria listed in the requirements document. |
Beta Was this translation helpful? Give feedback.
-
|
@javeedf cool, thanks, will take a look ASAP |
Beta Was this translation helpful? Give feedback.
-
|
Thank you for your submission, @javeedf . The items in the checklist can be in one of the following states:
Feel free to ask any questions and provide feedback. Thanks in advance for your understanding. For details about the following points, refer to the Collection Requirements. Public availability and communication:
Standards and documentation:
Collection management:
Tests:
|
Beta Was this translation helpful? Give feedback.
-
|
@tadeboro @Andersson007 Addressed all your comments. please review |
Beta Was this translation helpful? Give feedback.
-
|
I did my final review, and as far as I am concerned, it is ready to become part of the Ansible community package once a new release with all the fixes from the review process is released. Thank you, @javeedf, for persevering! |
Beta Was this translation helpful? Give feedback.
-
|
@javeedf cool, could you please release the fixed version as @tadeboro suggested. Thanks! |
Beta Was this translation helpful? Give feedback.
-
|
Done @Andersson007 |
Beta Was this translation helpful? Give feedback.
-
|
@javeedf thanks! Note for the other steering committee members: I didn't review CI part in detail. |
Beta Was this translation helpful? Give feedback.
-
|
I agree with @tadeboro and @Andersson007 that this collection is ready for inclusion! There's one detail though that must be fixed: the |
Beta Was this translation helpful? Give feedback.
-
|
(I don't mind if this is only fixed in a patch release after the deadline though, it's minor enough for that :) ) |
Beta Was this translation helpful? Give feedback.
-
|
@tadeboro @Andersson007 @felixfontein When shall we expect the collection included build for our review? |
Beta Was this translation helpful? Give feedback.
-
|
@javeedf The final decisions about inclusion will happen tomorrow (2021-04-28T18:00Z) during the community meeting. As for the build, we will not build the collection artifact ourselves. When the Ansible package is built, we just download the build artifacts from Ansible Galaxy. So the final artifact included in the ansible package will be built by you. IIRC, the 4.0.0-beta1 version of ansible will already contain all included collections so you will be able to test this in a few days. You will then have almost three weeks to find any issues before the final Ansible 4.0.0 is released. See the https://docs.ansible.com/ansible/devel/roadmap/COLLECTIONS_4.html#release-schedule for more information. |
Beta Was this translation helpful? Give feedback.
-
|
This collection has been prepared for inclusion in Ansible 4 in this PR: ansible-community/ansible-build-data#66 |
Beta Was this translation helpful? Give feedback.
-
|
The Congratulations! |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Include dellemc.enterprise_sonic into the upcoming Ansible community release. We have already published it into ansible automation hub. all the requirements must have met. please validate and let us know
Galaxy: https://galaxy.ansible.com/dellemc/enterprise_sonic
Repository: https://github.com/ansible-collections/dellemc.enterprise_sonic
Beta Was this translation helpful? Give feedback.
All reactions