Add a hook to run shellcheck on all "runs" steps in melange pipelines

[dannf]

No description provided.

[AmberArcadia]

Love the idea, and thanks for the implementation! I'm curious about setup.py and setup.cfg, such as if they should be in a subfolder or shared between multiple hooks. Perhaps we could build some documentation here too.

[smoser]

This looks really well done.
my only thoughts are

1. python dependencies - how do you assert a sane environment ? you also have to somehow deliver / assert shellcheck. (maybe you've solved that and I just don't understand. seems like a company that builds containers here could help.)
2. would having melange do this be sane? that gets you out having to deliver the python stuff at least. a 'melange check' command could be good and easily callable.

[AmberArcadia]

Re # 1: Pre-commit does handle setting up a Python environment and managing dependencies. Some more info here: https://pre-commit.com/#python

This also leads into my question a bit, seems we will want our Python hook stuff to be at the top level.

The hook repository must be installable via pip install . (usually by either setup.py or pyproject.toml)

[dannf]

This looks really well done. my only thoughts are

1. python dependencies - how do you assert a sane environment ? you also have to somehow deliver / assert shellcheck. (maybe you've solved that and I just don't understand. seems like a company that builds containers here could help.)

pre-commit provides a hook for making shellcheck available in the venv it instantiates. We'll want to add this to each repos .pre-commit-config.yaml ahead of this hook:

  - repo: https://github.com/shellcheck-py/shellcheck-py
    rev: v0.10.0.1 # v0.10.0.1
    hooks:
      - id: shellcheck

2. would having melange do this be sane? that gets you out having to deliver the python stuff at least. a 'melange check' command could be good and easily callable.

Perhaps. I went with pre-commit because we already use it, and it has all of this infra to do this stuff already.

[dannf]

Love the idea, and thanks for the implementation! I'm curious about setup.py and setup.cfg, such as if they should be in a subfolder or shared between multiple hooks. Perhaps we could build some documentation here too.

Yeah, I think this is required for python hooks, which I see you've also discovered below. fwiw, I used the upstream pre-commit-hooks repo as my template, and just trimmed out what I didn't need.

[dannf]

This looks really well done. my only thoughts are

1. python dependencies - how do you assert a sane environment ? you also have to somehow deliver / assert shellcheck. (maybe you've solved that and I just don't understand. seems like a company that builds containers here could help.)

pre-commit provides a hook for making shellcheck available in the venv it instantiates. We'll want to add this to each repos .pre-commit-config.yaml ahead of this hook:

  - repo: https://github.com/shellcheck-py/shellcheck-py
    rev: v0.10.0.1 # v0.10.0.1
    hooks:
      - id: shellcheck

Even though this is an easy option, I do like the hint of using a guarded container instead. I'll experiment with that.

[dannf]

This looks really well done. my only thoughts are

1. python dependencies - how do you assert a sane environment ? you also have to somehow deliver / assert shellcheck. (maybe you've solved that and I just don't understand. seems like a company that builds containers here could help.)

pre-commit provides a hook for making shellcheck available in the venv it instantiates. We'll want to add this to each repos .pre-commit-config.yaml ahead of this hook:

  - repo: https://github.com/shellcheck-py/shellcheck-py
    rev: v0.10.0.1 # v0.10.0.1
    hooks:
      - id: shellcheck

Even though this is an easy option, I do like the hint of using a guarded container instead. I'll experiment with that.

I've now added a --shellcheck command that default to a docker run of the upstream container. If this seems reasonable, we can transition to a guarded container.

[dannf]

for some reason this PR is wedged.
I pushed another copy here - #2