Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: add a shellcheck job for all our shell scripts #86

Merged
merged 10 commits into from
Feb 16, 2024

Conversation

hnez
Copy link
Member

@hnez hnez commented Dec 12, 2023

Pull Request #85 has brought the topic of shell scripts in meta-lxatac not passing shellcheck to my attention.

I consider even a shell script that passes shellcheck performing like the author intended a happy coincidence. One that does not pass shellcheck working correctly is a full blown miracle.

Make sure to check all scripts with shellcheck for each pull request.

Todo before merging:

  • Make sure the CI job runs without errors before merging this PR.

@hnez hnez requested a review from Emantor December 12, 2023 13:39
@hnez hnez self-assigned this Dec 12, 2023
@hnez hnez force-pushed the shellcheck-ci branch 3 times, most recently from d59fc3b to 02ac4c7 Compare December 12, 2023 14:06
@hnez hnez force-pushed the shellcheck-ci branch 4 times, most recently from 9f28c95 to b7e16c6 Compare December 15, 2023 15:00
@hnez
Copy link
Member Author

hnez commented Dec 15, 2023

The remaining issues are:

In meta-lxatac-software/recipes-rust/tacd/files/tacd-failsafe.sh line 6:
gpioset $(gpiofind DUT_PWR_EN)=1
        ^--------------------^ SC2046 (warning): Quote this to prevent word splitting.
          ^-----------------^ SC2312 (info): Consider invoking this command separately to avoid masking its return value (or use '|| true' to ignore).


In meta-lxatac-software/recipes-rust/tacd/files/tacd-failsafe.sh line 7:
gpioset $(gpiofind DUT_PWR_DISCH)=0
        ^-----------------------^ SC2046 (warning): Quote this to prevent word splitting.
          ^--------------------^ SC2312 (info): Consider invoking this command separately to avoid masking its return value (or use '|| true' to ignore).


In meta-lxatac-software/recipes-devtools/tac-gadget/files/gadget-common.sh line 19:
    elif [[ -s "${MAINDIR}"/*/UDC ]]; then
               ^----------------^ SC2144 (error): -s doesn't work with globs. Use a for loop.

The first two of these are fixed in #88. The last one requires some thinking and the fix is not as mechanical as the others.

@hnez hnez changed the base branch from mickledore to nanbield January 26, 2024 09:15
This change is less mechanic than the previous tac-gadget shellcheck
change and required some more thinking, which is why it is split
out from the rest.

Signed-off-by: Leonard Göhrs <[email protected]>
@hnez hnez marked this pull request as ready for review January 26, 2024 09:23
@hnez hnez assigned Emantor and unassigned hnez Jan 26, 2024
@hnez
Copy link
Member Author

hnez commented Feb 2, 2024

Hi @Emantor,

do you think you could squeeze in a review for this PR? Otherwise I could try finding another shell scripting expert.

@hnez
Copy link
Member Author

hnez commented Feb 15, 2024

Hi @Emantor,

this is a rather large PR with no user-facing improvements, so I am fine with this not being on the fast track.
But it would be cool to get it merged.

@hnez hnez merged commit 3771024 into linux-automation:nanbield Feb 16, 2024
3 checks passed
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.

2 participants