Inspired by https://gerrit.wikimedia.org/r/c/operations/puppet/+/405594 I think we should stop introducing source code expanded from erb templates, at least for these reasons:
Implementation wise: a CI check that detects new files being introduced and fails if it is source code + erb (e.g. .(py|sh|rb|pm).erb
fully support this
In addition to checking file extensions we could also check for presence of a shebang #! on the first line of .erb files.
I fully agree with the principle, but I have to admit that I'm also guilty as charged as I've recently add a few lines wrapper bash script that is dependent of django variables, and I'm wondering now if that could be considered an exception or we should go the extra mile and set up a config file for it and have the script just read it. See it here:
https://github.com/wikimedia/puppet/blob/production/modules/profile/templates/debmonitor/server/run_django_command.sh.erb
What is your thought?
This seems more of a discussion / should be added to the wikitech page on Puppet coding standards.. but as a ticket i'm not sure how actionable it is. It would be resolved once we stop doing something. That seems hard to decide when it can actually be closed.
It is a good counter example for the definition of "code", I like the simplicity of being able to run the wrapper without knowing anything else. An automated check seems less attractive and more a guideline like @Dzahn suggested might be more appropriate to apply some human judgement instead.
This seems somewhat but not exactly a duplicate of T254480. The other ticket would be resolved once we have removed all cases of it from the repo.
Then this ticket would be still here to prevent it from happening again. So does it mean this is about making jenkins-bot vote -1 if it detects such an attempt?
It'd be somewhat hard to detect new attempts I think, I'm resolving this in favor of T254480 et al.