-
Notifications
You must be signed in to change notification settings - Fork 11
Add initial version of a coding guide #19
base: master
Are you sure you want to change the base?
Conversation
lgtm |
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.
lgtm
GUIDE.md
Outdated
expanded only when `__timesync_chrony_version` is used, and if it is | ||
after we collect the fact, everything is fine. | ||
|
||
(Beware though, `set_facts` expands variables, because it is is like a |
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.
^is is^is^
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.
Thank you @nhosoi. Corrected.
For more in-depth explanations than in the coding standards.
Can this be merged? |
properly. | ||
|
||
Another place where this is used is here: | ||
https://github.com/linux-system-roles/timesync/blob/9050dd95314406236bc8869eac2307a6fa932edc/vars/main.yml#L1 |
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.
Make this a link for readability.
to the task, so the variable would be unset both before and after the | ||
task. | ||
See | ||
https://github.com/ansible/ansible/issues/66714 |
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.
Make these into links for readability.
|
||
[2] Another well-known language with this style of variable expansion | ||
is `make`: | ||
https://www.gnu.org/software/make/manual/html_node/Flavors.html |
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.
Make this a link for readability.
something like `role_name` to another role and use it to reference to | ||
the calling role, because the variable will only get expanded in the | ||
called role to something else - the variable represents the currently | ||
executing role. https://github.com/ansible/ansible/issues/10374 |
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.
Make this a link for readability.
Variables set on the `roles` or `import_role` statement appear like | ||
parameters, but actually they are made accessible to the whole | ||
playbook run, just like role variables set in `vars/` - therefore can | ||
influence further role invocations. |
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 don't know if it's worth notiing that this can be made to happen with include_role too: https://docs.ansible.com/ansible/latest/modules/include_role_module.html#parameter-public
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'll try that - does it really have the same effect on the variables given in the vars
keyword to include_role
as in the case of import_role
? The effect can't be exactly the same, because variables given on import_role
are visible even before this task, while public
makes the variables visible only after the include_role
task.
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.
Ah, good point.
influence further role invocations. | ||
|
||
Example: | ||
``` |
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.
Can we toss "```yaml" here for syntax highlighting?
For more in-depth explanations than in the coding standards.
File name is up to discussion.