Open issues on collections that use booleans to recommend true/false #60
Comments
|
List of collections (Ansible 7 based). Each one should be checked to see if they use booleans and if so, open an issue in that repository:
|
|
See ansible-collections/amazon.aws#1041 for an example on how to open a collection issue and what to include. |
|
What I did: |
|
community.aws is done already - ansible-collections/community.aws#1420 (no need to open issue) |
|
community.crypto, community.dns, community.docker, community.hrobot, community.interal_test_tools (not listed here), community.routeros, and community.sops are also done. community.general is partially done; I'm sure I missed some cases, but the bulk should be done (I hope). |
|
fyi: I've created an issue in community.mysql and I'm keeping it open advertising as an easyfix for newcomers |
|
checking against ansible-build-data/7/collection-meta.yaml i noticed 4 missing collections: Should they be notified as well? |
|
yeah for sure. Thanks for checking! I updated the checklist with those four. |
|
So .. double and triple checked, I'm ready to launch the issue strike against 96 collections, based on unchecked collections under #60 (comment) (except community.sops which should be checked) using this script Only exception is When I get a second yay for this count (96) and this exclusion list, I'll go nuclear! |
|
Issue for |
|
@kristianheljas that's great! Can I beg you for a couple of enhancements? No biggie if that's not possible. I 'think' I understand the script well enuf to at least edit it and ask for feedback/help in going nuclear on those other collection wish-list items. This is great stuff! |
|
@samccann, no need to beg - happy to contribute! :)
P.S. This "exlusion list" actually records all the issue links it creates, and populating it before running will make it ignore these collections. |
|
@kristianheljas @samccann are we talking about notifying maintainers about the doc standard change? If yes, I would also suggest putting a link to the doc standards to the issues, what do you think?
Do you mean a) how to notify maintainers or b) adding this in the collection requirement?
|
|
@Andersson007 I don't think I can link to docs.ansible.com, since the core team is not on board with this decision, see ansible/ansible#77581 (comment), so the docsite shows yes/no values. @samccann Do you happen to know if this standard is documented somewhere besides the voting topic? At a glance, I didn't find this in collection requirements. |
|
@kristianheljas @samccann i believe news-for-maintainers is to announce stuff relevant for collection maintainers, not for core maintainers. |
|
Given that there is now a Community decision ansible-community/community-topics#116 and antsibull-docs now converts over to true/false ansible-community/antsibull-docs#19 if it's not documented, we should update the documentation (somewhere) Additionally, if possible add a linting rule that prefers the strings "true"/"false". My understanding is that we can update the collections linting ruleset without updating the core ruleset (@felixfontein would such a linting rule be possible? - are they still strings by the time the linter sees them?) |
|
@tremble ansible-test uses PyYAML to load the YAML, which converts both What would be possible to adjust the yamllint config that is used to validate YAMLs in collections. But I'm pretty sure core won't want to allow this, since the yamllint config used is very, very lenient, and I don't think they want to use it to force any convention on users. But I guess we could ask them... |
|
The closest we have for documentation is at https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html#boolean-variables @Andersson007 I 'think' most of the requirements for a valid collection are still in https://github.com/ansible-collections/overview/blob/main/collection_requirements.rst not in docs.ansible.com yet. As for core not being onboard, they would not support something that forces it (Felix's prior comment I think) but are no longer against updating docs (or docs within collections) to use |
|
The best I found collection requirements is a link to Development conventions, which suggests But .. that's really great that the core team is ready to accept this change in the docs! @samccann @Andersson007 Should we postpone this mass-notification until we have some docs/requirements to support this or send these based on the voting topic alone to get things going in parallel? And are the mass-issues necessary if we announce it on news-for-maintainers? |
|
@kristianheljas I see value in opening issues as a reminder. We have a number of enhancements like this that collection owners aren't acting on, so this seems like a way to remind them directly. But I'm open to other views on this one. As for changing the docs, the module development guidelines should be updated. Currently it says: It should say something like: |
|
@Andersson007 @tremble @Andersson007 - we were going to do the batch issue creation as soon as the docs are updated to say folks should document booleans as true/false. Scream now if you're against this batch issue creation against all the collections that haven't done this already... :-) |
|
Created a PR for the guidelines here and updated the issue template. Feel free to make any applicable modifications to the template. Once the PR has made it's way to docs.ansible.com and no-one has complained, I will:
|
|
@samccann, issues have been launched! Out of 96 collections to notify, there were 2 exceptions:
Links for 94 issues created can be found in this gist (and above this comment, hehe) |
|
@wojtek0806, we wanted to open an issue to f5_modules.f5networks about collection standards change but it appears you do not have an active issue tracker (manifest points to GH and GH issues are disabled this repo). Can you please have a look and update the manifest or open the GH issue tracker? Also, here is the notification we wanted to send:
|
|
I fear that the answer will be "no", but anyway: Are there any scripts or tools to help maintainers with this, at least to find out what needs to be changed? And another thing: We use ansible-network/collection_prep in community.vmware to create the docs. I might be wrong, but it looks to me that it uses |
|
@mariolenz can you open an issue on that collection_prep? I'm not familiar with it but sounds like it's not following along on the true/false change. |
@samccann ansible-network/collection_prep#81 If anyone can point me to a better way to create the docs, feel free to do it. I'm not above replacing OK, we've dealt with my second question. The answer to my first one is... "no"? |
|
@mariolenz I don't think there is, the best i can think of right now is something like Regardless, if you are able to only reflect this in the source docstrings, thats a big win as well as it bring consistency to the ansible docsite, which i think was the main driving force behind all this. Thanks for opening the issue in collection_prep! |
This is my approach, too. I'll do my best, but if you don't have a tool to check this it's only "best effort" from my side. I hope to fix this, but as I've said: Only best effort ;-) |
|
If you use ansible-test devel from a checkout, you can apply the following diff to check at least some things: diff --git a/test/lib/ansible_test/_util/controller/sanity/yamllint/config/default.yml b/test/lib/ansible_test/_util/controller/sanity/yamllint/config/default.yml
index 45d8b7adcf..5ca29b7bbe 100644
--- a/test/lib/ansible_test/_util/controller/sanity/yamllint/config/default.yml
+++ b/test/lib/ansible_test/_util/controller/sanity/yamllint/config/default.yml
@@ -16,4 +16,8 @@ rules:
new-line-at-end-of-file: disable
new-lines: {type: unix}
trailing-spaces: disable
- truthy: disable
+ truthy:
+ level: error
+ allowed-values:
+ - 'true'
+ - 'false'
diff --git a/test/lib/ansible_test/_util/controller/sanity/yamllint/config/modules.yml b/test/lib/ansible_test/_util/controller/sanity/yamllint/config/modules.yml
index da7e604999..d1add23b51 100644
--- a/test/lib/ansible_test/_util/controller/sanity/yamllint/config/modules.yml
+++ b/test/lib/ansible_test/_util/controller/sanity/yamllint/config/modules.yml
@@ -16,4 +16,8 @@ rules:
new-line-at-end-of-file: disable
new-lines: {type: unix}
trailing-spaces: disable
- truthy: disable
+ truthy:
+ level: error
+ allowed-values:
+ - 'true'
+ - 'false'
diff --git a/test/lib/ansible_test/_util/controller/sanity/yamllint/config/plugins.yml b/test/lib/ansible_test/_util/controller/sanity/yamllint/config/plugins.yml
index 6d41813787..d1add23b51 100644
--- a/test/lib/ansible_test/_util/controller/sanity/yamllint/config/plugins.yml
+++ b/test/lib/ansible_test/_util/controller/sanity/yamllint/config/plugins.yml
@@ -11,9 +11,13 @@ rules:
empty-lines: disable
hyphens: disable
indentation: disable
- key-duplicates: disable
+ key-duplicates: enable
line-length: disable
new-line-at-end-of-file: disable
new-lines: {type: unix}
trailing-spaces: disable
- truthy: disable
+ truthy:
+ level: error
+ allowed-values:
+ - 'true'
+ - 'false' |
|
(Please note that this will flag most GitHub workflows that contain |
|
Looking at community.aws I've noticed that a couple of regressions have crept it. While we can whack-a-mole these, unless there's something we can add to CI, they will keep creeping back in. |
Thanks @felixfontein, this really helps! |
I really wish there was a way for collections to override some of these default settings. Some of the disabled rules would actually be nice to enable
You can also tweak the workflow YAML files to tell yaml-lint to ignore the lines (and still grumble at other truthy-ness issues) https://yamllint.readthedocs.io/en/stable/disable_with_comments.html |
This also flags something like I'll ignore this, just FYI. |
|
@mariolenz - what do you use the docs for when you use collection_prep? Is it just to test that the docs generate? OR are you putting them up on some website somewhere? Galaxy beta (aka GalaxyNG) displays module/plugin docs now so there's less need for collection owners to generate module rst files and keep them in their repo. |
Well, but So the warning is correct and you should always put |
Thanks! I didn't know this, I'll fix it. |
We generate the docs and put them into the docs directory since the VMware modules have been moved out of ansible and into a dedicated collection. We don't push them to any other website. How do tother collections handle module documentation? If it's the default to not have this in the repo, I can change it. |
|
@mariolenz - all those module docs are visible today at https://beta-galaxy.ansible.com/ui/repo/published/community/vmware/content/module/vcenter_domain_user_group_info/ for example. So once Galaxy NG is ..beyond beta, you can probably stop using the collection_prep and just point to your collection on GalaxyNG for the module docs. |
|
@samccann I could also link to docs.ansible.com, I just never did. And since we're discussing moving to a new domain(ansible-community/community-topics#201) and Galaxy NG is still beta, I'd like to wait a bit before I decide what to do. Anyway, I think I've fixed this in most places in the DOCUMENTATION and EXAMPLES blocks. |
|
The purpose of this issue was to create the reminder issues in each collection, so going to close this out as done. Thanks @kristianheljas for the script help and everyone else for the help! |
As part of the community decision to standardize on true/false for boolean values, We need to open issues on the collections that use these to recommend they follow this practice.
Using this issue to track which collections we open the issues on. NOTE we cannot mandate the collection owner use true/false so we will only track that we opened the issues with that recommendation.
The text was updated successfully, but these errors were encountered: