Skip to content

Conversation

jplehr
Copy link
Contributor

@jplehr jplehr commented May 12, 2025

No description provided.

jplehr added 4 commits May 12, 2025 09:33
This adds an initial (crude) shellcheck workflow.

This can be refined much more.
- Use shellcheck directly from maintainer github.
- Filter which files to check.
- Come up with set of guidelines / rules for errs/warns.
Use find + xargs to make the job fail in case shellcheck found issues.
@jplehr jplehr requested review from jtb20 and Kewen12 May 12, 2025 14:52
@jplehr jplehr changed the title Add shellcheck workflow [AOMP] Add shellcheck workflow May 12, 2025
@jplehr
Copy link
Contributor Author

jplehr commented May 13, 2025

This depends on #1252 and makes only sense once that has landed.

@jplehr jplehr requested a review from dpalermo May 20, 2025 13:56
@jplehr
Copy link
Contributor Author

jplehr commented Jun 2, 2025

I think we want to wait for #1463 to land and then use this wrapper instead of plain shellcheck in this workflow.

@jtb20
Copy link
Contributor

jtb20 commented Jun 2, 2025

As it is, the aomp-shellcheck script automatically modifies the files given as arguments in some circumstances (i.e. if they have failing checks in the "patchable" check list), which is probably not desirable for the GitHub workflow case.

@jplehr
Copy link
Contributor Author

jplehr commented Jun 2, 2025

Good point. Should there be a --check-only option or so that would point out issues and show how to use our wrapper to fix them?

@jtb20
Copy link
Contributor

jtb20 commented Jun 2, 2025

I think so, something like that.

@jtb20
Copy link
Contributor

jtb20 commented Jun 4, 2025

I've added a new --check-only option in the version up on #1463 now.


- name: Run shellcheck
run: |
find . -name "*.sh" | xargs shellcheck -S info {} \;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The syntax is wrong, which must be

Suggested change
find . -name "*.sh" | xargs shellcheck -S info {} \;
find . -name "*.sh" -exec shellcheck -S info {} +

or

Suggested change
find . -name "*.sh" | xargs shellcheck -S info {} \;
find . -name "*.sh" | xargs shellcheck -S info

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants