-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Added docs Inventory Guide. #10239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added docs Inventory Guide. #10239
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! I've added some first comments below.
I think @felixfontein 's line of thought makes sense, these guides should be put together as |
I haven't checked all files in detail yet, I mainly skimmed over it and added comments for some things that seemed to happen more than once / I noticed on the first pass. I'll take a closer look later :) |
Okie, I'll do the same - but not today |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple of comments
.. code-block:: bash | ||
|
||
shell> cat /zroot/iocage/jails/srv_1/root/etc/dhclient-exit-hooks | ||
case "$reason" in | ||
"BOUND"|"REBIND"|"REBOOT"|"RENEW") | ||
echo $new_ip_address > /var/db/dhclient-hook.address.$interface | ||
;; | ||
esac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other .rst files, to showcase the contents of yaml files, you created two code-blocks, one "console" with the cat
command and the other one with the content. Here it is all in one block. I believe both ways to be OK, but whichever one you want to use, apply it consistently to all cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I consistently apply whatever works. The single bash
block works fine here. This is not the case with yaml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely not a generic "Inventory Guide" but rather an "iocage inventory guide". I think it should be named accordingly. Maybe a new section should be opened, like "Technology Guide" or something similar - I don't think it fits well in any of the existing sections.
I've already told you:
This is up to you. I'm only interested in the "inventory iocage" guide. Feel free to place it wherever you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I consistently apply whatever works. The single
bash
block works fine here. This is not the case withyaml
.
But the two cases are actually just one case, which is showing content of a file with cat
. I reckon they should be formatted the same way. IMHO the current disposition is not a good editorial choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely not a generic "Inventory Guide" but rather an "iocage inventory guide". I think it should be named accordingly. Maybe a new section should be opened, like "Technology Guide" or something similar - I don't think it fits well in any of the existing sections.
I've already told you:
This is up to you. I'm only interested in the "inventory iocage" guide. Feel free to place it wherever you'd like.
And as I wrote in that comment, the suggested placement is a bit off. That being said, the second half of that comment was intended to be an invitation to a discussion, to you, to @felixfontein and anyone else in the community who might want to chip in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed problematic pygments to console
. I won't use ansible-output
because the playbook outputs are YAML. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, the ansible-output
lexer is in https://github.com/ansible-community/ansible-pygments/blob/main/src/ansible_pygments/lexers.py#L44-L214. If someone wants to create a lexer for Ansible output with YAML, that's the place where it belongs to. Not sure how hard that would be though, since handling mixing YAML, regular Ansible output, and diff output isn't trivial...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... the second half of that comment was intended to be an invitation to a discussion
I created this PR. I don't need an invitation.
I did that out of politeness to you and to others, not out of your necessity.
That being said, please create a new section for your guide - I suggested "Technology Guide" but you may prefer something else. Just keep in mind, as mentioned before, that "Inventory Guide" is misleading because this is not about inventories in general, but rather about one single technology that happens to provide an inventory.
And please mark the command line as console
and the output as bash
for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... create a lexer for Ansible output with YAML
JSON is a subset of YAML.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but not the other way around :) The advantage of a JSON object or list is that it's clear when it's done (assuming it doesn't have syntax errors). For YAML that's definitely not the case, which makes an Ansible output parser for YAML output harder than for JSON output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional comments and responses.
.. code-block:: bash | ||
|
||
shell> cat /zroot/iocage/jails/srv_1/root/etc/dhclient-exit-hooks | ||
case "$reason" in | ||
"BOUND"|"REBIND"|"REBOOT"|"RENEW") | ||
echo $new_ip_address > /var/db/dhclient-hook.address.$interface | ||
;; | ||
esac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... the second half of that comment was intended to be an invitation to a discussion
I created this PR. I don't need an invitation.
I did that out of politeness to you and to others, not out of your necessity.
That being said, please create a new section for your guide - I suggested "Technology Guide" but you may prefer something else. Just keep in mind, as mentioned before, that "Inventory Guide" is misleading because this is not about inventories in general, but rather about one single technology that happens to provide an inventory.
And please mark the command line as console
and the output as bash
for consistency.
I changed the pygment to sh. Quoting man dhclient-script:
|
For the record: The file inventory_guide.rst serves the purpose of inventory_guide_iocage.rst temporary integration. Please remove the BOTMETA entry after you decide where to place it
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't go through everything, but here's a bunch of more comments:
For the record:
|
Yep, sorry, it's |
a6e0c00
to
1c91a91
Compare
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
3b7c5d2
to
63bcde7
Compare
Backport to stable-11: 💚 backport PR created✅ Backport PR branch: Backported as #10395 🤖 @patchback |
* Added docs Inventory Guide. * Errata docs Inventory Guide. * Fix docs Inventory Guide error: use ASCII quotes. * Fix docs Inventory Guide various lint errors. * Added docs Inventory Guide BOTMETA entries. * Fix docs Inventory Guide lint errors: trailing whitespace * Fix docs Inventory Guide lint errors: force yaml pygment * Fix docs Inventory Guide lint errors: No way to force yaml pygment in code-block * Update docs/docsite/rst/inventory_guide_iocage.rst Co-authored-by: Felix Fontein <[email protected]> * Update docs/docsite/rst/inventory_guide_iocage_aliases.rst Thank you for the explanation! Co-authored-by: Felix Fontein <[email protected]> * Update docs/docsite/rst/inventory_guide_iocage_aliases.rst Co-authored-by: Felix Fontein <[email protected]> * Updated docs Inventory Guide. * Problematic pygments changed to 'console'. * Update docs/docsite/rst/inventory_guide_iocage_hooks.rst Update docs/docsite/rst/inventory_guide_iocage_properties.rst Update docs/docsite/rst/inventory_guide_iocage_hooks.rst Co-authored-by: Alexei Znamensky <[email protected]> * Put dhclient-exit-hooks into the sh code-block. * Fix the code-block. * Update docs/docsite/rst/inventory_guide_iocage.rst Update docs/docsite/rst/inventory_guide_iocage_aliases.rst Update docs/docsite/rst/inventory_guide_iocage_basics.rst Co-authored-by: Felix Fontein <[email protected]> * Remove tabs. * Update docs/docsite/rst/inventory_guide_iocage_basics.rst Co-authored-by: Felix Fontein <[email protected]> * Indent the note block. * Update docs/docsite/rst/inventory_guide_iocage_hooks.rst Update docs/docsite/rst/inventory_guide_iocage_dhcp.rst Update docs/docsite/rst/inventory_guide_iocage_hooks.rst Co-authored-by: Felix Fontein <[email protected]> * Fix ansval. * Add guide_iocage.rst and inventory_guide_iocage*.rst * Fix 'disallowed language sh found'. * Remove note block. * Remove include which triggers a bug in rstcheck. * Update docs/docsite/extra-docs.yml Update docs/docsite/rst/iocage_inventory_guide_basics.rst Update docs/docsite/rst/iocage_inventory_guide_dhcp.rst Update docs/docsite/rst/iocage_inventory_guide_hooks.rst Update docs/docsite/rst/iocage_inventory_guide_properties.rst Update docs/docsite/rst/iocage_inventory_guide_tags.rst Update docs/docsite/rst/iocage_inventory_guide_hooks.rst Update docs/docsite/rst/iocage_inventory_guide_properties.rst Co-authored-by: Felix Fontein <[email protected]> * Put man iocage quotation into the text code block. --------- Co-authored-by: Felix Fontein <[email protected]> Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 563b29e)
…10395) Added docs Inventory Guide. (#10239) * Added docs Inventory Guide. * Errata docs Inventory Guide. * Fix docs Inventory Guide error: use ASCII quotes. * Fix docs Inventory Guide various lint errors. * Added docs Inventory Guide BOTMETA entries. * Fix docs Inventory Guide lint errors: trailing whitespace * Fix docs Inventory Guide lint errors: force yaml pygment * Fix docs Inventory Guide lint errors: No way to force yaml pygment in code-block * Update docs/docsite/rst/inventory_guide_iocage.rst * Update docs/docsite/rst/inventory_guide_iocage_aliases.rst Thank you for the explanation! * Update docs/docsite/rst/inventory_guide_iocage_aliases.rst * Updated docs Inventory Guide. * Problematic pygments changed to 'console'. * Update docs/docsite/rst/inventory_guide_iocage_hooks.rst Update docs/docsite/rst/inventory_guide_iocage_properties.rst Update docs/docsite/rst/inventory_guide_iocage_hooks.rst * Put dhclient-exit-hooks into the sh code-block. * Fix the code-block. * Update docs/docsite/rst/inventory_guide_iocage.rst Update docs/docsite/rst/inventory_guide_iocage_aliases.rst Update docs/docsite/rst/inventory_guide_iocage_basics.rst * Remove tabs. * Update docs/docsite/rst/inventory_guide_iocage_basics.rst * Indent the note block. * Update docs/docsite/rst/inventory_guide_iocage_hooks.rst Update docs/docsite/rst/inventory_guide_iocage_dhcp.rst Update docs/docsite/rst/inventory_guide_iocage_hooks.rst * Fix ansval. * Add guide_iocage.rst and inventory_guide_iocage*.rst * Fix 'disallowed language sh found'. * Remove note block. * Remove include which triggers a bug in rstcheck. * Update docs/docsite/extra-docs.yml Update docs/docsite/rst/iocage_inventory_guide_basics.rst Update docs/docsite/rst/iocage_inventory_guide_dhcp.rst Update docs/docsite/rst/iocage_inventory_guide_hooks.rst Update docs/docsite/rst/iocage_inventory_guide_properties.rst Update docs/docsite/rst/iocage_inventory_guide_tags.rst Update docs/docsite/rst/iocage_inventory_guide_hooks.rst Update docs/docsite/rst/iocage_inventory_guide_properties.rst * Put man iocage quotation into the text code block. --------- (cherry picked from commit 563b29e) Co-authored-by: Vladimir Botka <[email protected]> Co-authored-by: Felix Fontein <[email protected]> Co-authored-by: Alexei Znamensky <[email protected]>
SUMMARY
Add Inventory (Plugin) Guide
ISSUE TYPE
COMPONENT NAME
NA
ADDITIONAL INFORMATION
NA