Repository navigation
ci: add shellcheck workflow for shell script linting - #16
Merged
Merged
Conversation
Add GitHub Actions workflow that runs shellcheck -S warning on shell scripts and bash tests. Includes repo-root .sh/.bash files and tests/ directory. Excludes PR-A2/B files (will be removed in PR-D #9). Includes tests/fakes/* scripts for comprehensive linting coverage. Runs shellcheck -S warning and bash tests/run.sh on each push/PR.
RyodoTanaka
commented
Oct 15, 2026
RyodoTanaka
left a comment
Member
Author
There was a problem hiding this comment.
レビュー(Opus): マージ可
Issue #9 §5/§6 の PR-C(shellcheck workflow)部分と、#14 レビューの CI 低指摘への対応を確認した。
確認できたこと
- 対象・除外が §5/§6 どおり: repo 直下の
*.sh/*.bash(-maxdepth 1なのでcan-utils/・openimu/は構造上入らない。! -pathは冗長だが無害)、-S warning。A2/B の 4 ファイル(canhat_fd_setup.bash・canhat_fd_rename.bash・enable_canhat_fd.sh・can_monitor.sh)は除外し、「PR-D で外す」コメントあり。 - 拡張子なしの fakes を拾う: 同じ条件で手元で試し、
tests/fakes/{docker,ip,journalctl,systemctl,mk_sysfs.sh}とtests/test_*.shの 4 本が対象になることを確認した。 - 失敗が job に伝わる: Actions の
run:は既定でbash -e -o pipefailなので、shellcheckが非 0 ならその場で step が落ちる(rc=$?以降には来ないが結果は同じ)。tests/run.shは失敗が 1 件でもあればexit 1で、step も落ちる。 - A2(#13)・B(#15) のマージ後: 直下に新しく入る
install.sh(A2)とcan_watchdog.sh(B)は除外されず対象になるが、両方ともshellcheck 0.11 -S warningで警告 0(rc=0)だった。新しいtests/test_*.shは対象になり、警告 0 は確認済み。B のcan_monitor.sh削除も除外判定は basename 比較なので問題ない。 - 安全: secrets は使っていない。外部 action は
actions/checkout@v4だけ。CI は緑(push と pull_request で 2 回)。
低(任意。PR-D でまとめて直してもよい)
permissions: contents: readを明記していない。リポジトリ/組織の既定が write だと、GITHUB_TOKEN に必要以上の権限が付く。tests/直下はtest_*で絞っているので、tests/run.shは対象外になる(§5 の「tests/」より狭い)。今は警告 0 なので、対象に足しても通る。tests/fakes/配下は拡張子を見ずにすべて渡している。シェルでないデータ(JSON の雛形など)を置くと誤検出で落ちる。逆に、can_diag 用の fakes をtests/diag_fakes/のような別ディレクトリに置くと対象から漏れる。fakes はtests/fakes/にシェルだけ置く、という運用を守るか、shebang で判定するとよい。on: [push, pull_request]なので、同じリポジトリ内の PR では 2 回走る。push をbranches: [develop, main]に絞ると重複がなくなる。ubuntu-latestの apt 版 shellcheck は手元の 0.11 より古い。ローカル(契約)と CI で結果がずれることがある。
クリティカル・高・中の指摘なし。マージ可。
RyodoTanaka
added a commit
that referenced
this pull request
Oct 15, 2026
Addresses the PR-A2 3rd review (pullrequestreview-5528306698) per coordinator decision: HAT+ is out of scope for Issue #9 §2b, so instead of fixing the --plus flag (which enabled canhat_plus_* *alongside* the FD units, reproducing the exact race the earlier C-new1 fix was meant to prevent - M-r3), remove it entirely. - install.sh never installs or enables canhat_plus_rename.service / canhat_plus_setup.service, under any flag or env var (CAN_TOOLS_INSTALL_PLUS and --plus are both gone). The canhat_plus_* scripts are still copied to /opt/can_tools (harmless, never run on their own). - README's "CAN HAT Plus" section now documents the manual setup instead (disable the FD units first, copy+enable the plus units by hand), and notes that can_watchdog (once it exists, PR-B) is FD-only for now and not meant to run on a HAT+ machine - flagged for PR-B to pick up. - L-r3a (optional, done): _wait_inactive now polls `systemctl show -p ActiveState --value` instead of `is-active`, since a running oneshot reports ActiveState=activating (is-active's exit status alone can't tell "still running" apart from "never ran"/ "finished"/"failed" the way checking the value for activating/deactivating can). - Merged origin/develop (c63da4d..78a6fd7, PR #16 shellcheck CI workflow) via a merge commit, no rebase. tests/test_install_idempotent.sh: Scenario 6 now asserts canhat_plus_* is never installed/enabled for any of install.sh's invocations (no args, a stray "--plus" - now just an ignored unrecognized argument - and "--check"); scenario 4's can_monitor assertion checks for the new `show -p ActiveState --value` call instead of `is-active`. can_tools_lib.bash and tests/fakes/ are untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Oct 15, 2026
RyodoTanaka
added a commit
that referenced
this pull request
Oct 15, 2026
This was referenced Oct 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
変更点
GitHub Actions ワークフロー
.github/workflows/shellcheck.ymlを追加しました。shellcheck -S warningでシェルスクリプトをリントbash tests/run.shでテスト実行Refs
ローカル実行結果
shellcheck -S warning
bash tests/run.sh
すべてのテストが通過しました。
🤖 Generated with Claude Code