infrastructure icon indicating copy to clipboard operation
infrastructure copied to clipboard

Set scripting/coding/style standards

Open Gorian opened this issue 4 years ago • 10 comments

Something I've often seen teams neglect but is important for long-term maintenance and ease of collaboration is standardizing on coding and style standards, like https://google.github.io/styleguide/shellguide.html for bash scripts, making sure that people aren't writing POSIX scripts but then using a #!/bin/bash shebang, or requiring python to comply with pep8, etc

Gorian avatar Dec 10 '20 21:12 Gorian

What are the automatic tools that could help enforce this? For shell scripts, running at least ShellCheck is probably a good idea.

oranenj avatar Dec 10 '20 22:12 oranenj

Agreed we need to get something formalised before we get too many contributions.

Highly recommend we get ansible-lint set up and make it a requirement on the Ansible side.

Generally pre-commit is not a bad idea as well but not sure how restrictive we want this to be.

Darkbat91 avatar Dec 10 '20 22:12 Darkbat91

+1 for ansible-lint

dpavlos avatar Dec 10 '20 23:12 dpavlos

As someone who helps to maintain some of the following, and writes a TON of Ansible code...

  • ansible-lint is THE linter for Ansible code and should definitely be applied
  • yamllint is THE linter for YAML in general and should also be applied
  • Molecule is a tester for Ansible playbooks and roles and we should think about wrapping this around important bits
  • ansible-test is the official test tool for Ansible modules and, likewise, should be wrapped around any custom modules we write
  • tox-ansible is a plugin for tox that brings together running most of the above on Ansible code

Putting all of them into pre-commit is not too difficult, either.

greg-hellings avatar Dec 11 '20 06:12 greg-hellings

ansible-lint GH Action: https://github.com/ansible/ansible-lint-action (PR https://github.com/rocky-linux/infrastructure/pull/24 merged) shellcheck GH Action: https://github.com/ludeeus/action-shellcheck yamllint GH Action: https://github.com/ibiqlik/action-yamllint (PR https://github.com/rocky-linux/infrastructure/pull/13 merged)

danielkubat avatar Dec 11 '20 12:12 danielkubat

I guess this is a good place to bring this up, as it is not part of our current ansible structure.

How do we want to handle collections? Specifically levering Galaxy collections https://galaxy.ansible.com/ so we are duplicating efforts that are already well maintained?

derekmpage avatar Dec 12 '20 06:12 derekmpage

I guess this is a good place to bring this up, as it is not part of our current ansible structure.

How do we want to handle collections? Specifically levering Galaxy collections https://galaxy.ansible.com/ so we are duplicating efforts that are already well maintained?

Our ansible structure emulates loosely the Fedora and CentOS ansible structure - We will be using ansible collections that are pulled as defined by the requirements.yml. For example, we are already defining the freeipa collection.

nazunalika avatar Dec 12 '20 06:12 nazunalika

+1 for ShellCheck. Standardising the way things are developed is crucial.

lisenet avatar Dec 17 '20 22:12 lisenet

I'd just like to point out that something like shellcheck and having a style guide are not necessarily the same thing - for example, shellcheck won't care if you comment your functions correctly, or whether you put do on the same line as a forloop or not, etc. but those sorts of things are still important for consistency in a large project - arguably, the correct placement of braces or whether then goes on the same line as if or the next is less important than tabs vs. spaces, but it's still important to define these things in the beginning rather than later when you realize it causes issues and then have to refactor old code.

Gorian avatar Dec 18 '20 00:12 Gorian

+1 for molecule and testinfra

christophedumont5 avatar Dec 18 '20 14:12 christophedumont5