redhat-cop
Recommendations for Jinja and roles
Add Jinja recommendations and some role text that was missed in previous PR.
This will likely conflict with 64,65; my expectation would be that those need to land first so I can rebase and clean this one up.
@ericzolf I know I'm too late for the call today but when folks have a chance this should be ready for review
Hi @ericzolf, would it be possible to get a re-review on this before the CoP meeting next week?
I'm fine with almost everything but still struggling with Design the interface focused on the functionality, not on the software implementation behind it. The message isn't either the issue, rather the example chosen. As much as I recommend not using tasks_from and rather splitting roles to make them more granular and fit for purpose (how many times do I see include_role: something_create/tasks_from: delete.yml which basically does the contrary of what the role's name says it does?), I still wouldn't recommend to have a front-end role with such a complex variables structure. I don't have a better example but perhaps the crowd has? If it needs be, I would prefer no example than this example, but we can discuss.
Why wouldn't you just use a simple variable structure? For example:
- hosts: all
gather_facts: false
tasks:
- name: Perform several actions
include_role: mycollection.run
vars:
var_1: 1
var_2: 2
var_3: 3
var_4: 4
If the point of the role is to hide the underlying implementation of the included roles, it seems like that should also extend to the variables. The example as it exists makes it look like the variables are just being passed through, which would expose the user to changes in those roles. You might need a bit more explanatory text since this example doesn't make it clear what the connection is to the underlying variables, but that kind of seems like what you'd want in this pattern.
@ericzolf Do you think gravesm's suggestion^ would be better? Or should we just get rid of the contrived examples entirely?
I resolved all outdated comments, hopefully everything is clear now (only two small formatting fixes). Sorry for the big delay, this MR review got swamped in daily life. Thanks for supporting this effort!
Approved yesterday in the CoP meeting, merging. Thanks for the effort (and the patience!).