gh-action to enable shell check in PR

[mbauhardt]

why

Running shell check against changed scripts within a PR is helpful to detect linting issues upfront.

what

Added a workflow which runs against all relevant shellscripts and fail when an error is detected.

[arcxio]

this is way too much infrastructure just to run shellcheck for my taste, but I'll let @biox be the judge

[mbauhardt]

@arcxio whats your recommendation except having nothing?

[arcxio]

pash actually has a workflow exactly for this purpose and it looks much simpler: https://github.com/dylanaraps/pash/blob/master/.github/workflows/main.yml can it be reused or is there something missing? (it doesn't seem to be working, though, I don't see any workflow runs in the repo... but maybe it's because it's archived) and if it were to be added, I'd say it would also need to run shfmt

[mbauhardt]

? this is the same, but without the path check. so in their action when changing for example the readme only, we would run this check as well which is not necessary. So they have less code, which is fine, but less accurate against the changeset. Anyway, if you think we are running the action independent of the change, I can remove the path check.

[arcxio]

what problem are you solving? if you're concerned with wasted resources on running redundant shellchecks, you instead fire up three additional containers on Ubuntu for every PR, including this "dorny/paths-filter" one made with typescript, which I bet wastes significantly more resources than shellcheck

[mbauhardt]

what problem are you solving?

Running shellcheck when shell-scripts were changed.

if you're concerned with wasted resources on running redundant shellchecks, you instead fire up three additional containers on Ubuntu for every PR, including this "dorny/paths-filter" one made with typescript, which I bet wastes significantly more resources than shellcheck

I don't get what your concern is. anyway, i'll remove the path check.

[arcxio]

can you switch to this action? it also runs shfmt and seems to be more popular and more actively maintained

[mbauhardt]

can you switch to this action? it also runs shfmt and seems to be more popular and more actively maintained

sure, let me take a look.

[mbauhardt]

would be helpful to specify what we want to achieve with that action.

A) I'm think good enough would be

• notify via label or comment on a PR via shellcheck (azohra/shell-linter@latest is doing this) when lint errors exists
• the developer hast to fix this manually and push the change, the action will check again
• we could provide a git-hook on push to do the same thing

B) a more complex achievement would be

• running a formatter to format the codebase
• if formatting changes are there commit and push the change automatically
• in this case AFAIK we have -- add a GH-TOKEN to be able to push -- the dev has to pull the changes if he/she wants to add another changes

I vote for A because it is simple and good enough. but sure, if we really want to have B I don't have strong emotional concerns against it.

BTW, formatting and linting are a different kind of shoes.

[arcxio]

if formatting changes are there commit and push the change automatically

correct me if I'm wrong but I think the action I've linked doesn't do that. it just checks that shfmt runs without errors, and PR's author has to run the formatter themselves if it doesn't.

[biox]

i honestly don't really want this project to depend on gha. it's simple enough that if we fuck it up, nbd someone can just push a fix. imo ci systems are for more-than-shell-script sized things. i appreciate the contribution @mbauhardt but i must politely decline.