ci: lint shell, workflows, TOML, Nix, and justfiles via nix devShell#1519
Conversation
Wires three new linters into the existing just check / just ci pipeline:
- shellcheck + shfmt over all 19 shell scripts (.github/scripts, go/,
kt/, swift/, infra/{apt,rpm}, rs/{libmoq,moq-ffi,moq-gst,scripts},
demo/throttle/enable).
- actionlint over .github/workflows/*.yml, exposed via the new
.github/justfile module so it parallels the per-area justfile pattern.
Tools come from nixpkgs via a new lintDeps group in flake.nix. The local
just check / just fix recipes guard each tool with `command -v` so they
skip silently when not on $PATH; just ci requires them (it always runs
inside `nix develop`).
.editorconfig gains a [*.sh] block locking in 4-space indent and
switch_case_indent so shfmt is deterministic across machines, plus
`ignore = true` for node_modules/target/dist/.venv so shfmt's recursive
walk doesn't pick up vendored files.
Fallout from the initial pass:
- shfmt reformatted all 19 scripts (mechanical: case bodies expanded
onto multiple lines, line-continuation `\` removed where `|` already
implies continuation).
- shellcheck found one real issue: dropped unused SCRIPT_DIR assignment
in swift/scripts/publish.sh.
- actionlint (via embedded shellcheck) flagged six workflow lines:
quoted $GITHUB_OUTPUT in docker.yml, refactored consecutive >>
redirects in release-brew.yml into a single `{...} >>` block, and
added targeted SC2016 disables for the four nfpm `envsubst` calls
(single-quoting the varlist is the correct invocation).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds editor and Taplo formatter configs, introduces a .github/justfile and lintDeps in flake.nix, inserts ShellCheck suppressions and grouped output writes in several GitHub workflows, and refactors many shell/build/package scripts and justfiles across Go, Kotlin, Rust, Swift, and infra for consistent CLI parsing, redirection, and explicit preflight/error guards while preserving existing behavior. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Extends the lintDeps group in flake.nix with:
- taplo for TOML formatting (Cargo.toml, deny.toml, pyproject.toml,
relay configs, etc.). Configured via .taplo.toml to match the existing
4-space indent, with column_width = 150 to avoid fighting cargo-sort
on long dep lines. exclude rules keep taplo out of node_modules,
target/, dist/, and .venv/.
- nixfmt (RFC-style) for flake.nix, nix/overlay.nix, and the relay
NixOS module. Used through `nix develop` -- the `formatter` field in
flake.nix (pkgs.nixfmt-tree) handles `nix fmt` independently and is
unchanged.
- just --fmt --unstable for every justfile in the repo. The flag is
still marked unstable upstream but the formatter is stable enough to
enforce; it normalizes tabs to 4-space indent across the 17 justfiles.
The same conditional pattern applies: just check / just fix guard each
new tool with `command -v` so local runs without `nix develop` skip
silently, while just ci runs them unconditionally.
rs/justfile passes --no-format to `cargo sort` so cargo-sort owns
dependency ordering and taplo owns whitespace/wrapping. Without this
they fought over `quinn = { ..., features = [...] }`.
Format-pass fallout, all mechanical:
- 11 justfiles indent-converted (tabs -> 4 spaces) and minor recipe
formatting from just --fmt.
- flake.nix and the two nix/ files reformatted by nixfmt (mostly
`with pkgs; [...]` expanded to a let-style with separate lines).
- rs/moq-native/Cargo.toml: one long features array stayed inline (now
under column_width = 150) and one was rewrapped multi-line.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@demo/boy/justfile`:
- Around line 76-80: The for-loop over "{{ dir }}"/* can iterate the literal "*"
when the directory is empty; add a guard before calling basename and running the
upload command in the loop (the block starting with for file in "{{ dir }}"/*;
do) — e.g., skip iteration unless the globbed path exists/is a regular file
(test with [ -e "$file" ] or [ -f "$file" ]), so only run key=$(basename
"$file") and bun wrangler r2 object put "rom-moq-dev/$key" --file "$file"
--remote for real files.
In `@demo/pub/justfile`:
- Around line 238-240: The download step writes to media/{{ name }}.mp4 but
doesn’t ensure the media directory exists; update the `@if` block in the Justfile
so it creates the directory (e.g., mkdir -p media) before running curl.
Specifically, modify the conditional around the media/{{ name }}.mp4 check in
the Justfile so that when the file is missing you first run a directory-creation
command, then curl the URL to media/{{ name }}.mp4.
- Around line 29-33: The loop over "{{ dir }}"/* can expand to a literal glob
when the directory is empty, causing failing uploads; modify the loop around the
variable "$file" (and the key=$(basename "$file") / bun wrangler r2 object put
invocation) to skip non-existent matches by adding a file-exists guard—either
enable nullglob behavior before iterating or add an if test inside the loop
(e.g., check that "$file" exists and is a regular file and continue if not) so
the upload command only runs for real files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d4c2738-8496-46d3-aa43-21622eb3bd28
📒 Files selected for processing (22)
.github/justfile.taplo.tomldemo/boy/justfiledemo/justfiledemo/pub/justfiledemo/relay/justfiledemo/sub/justfiledemo/web/justfileflake.nixgo/justfileinfra/apt/justfileinfra/justfileinfra/rpm/justfilejs/justfilejustfilekt/justfilenix/modules/moq-relay.nixnix/overlay.nixpy/justfilers/justfilers/moq-native/Cargo.tomlswift/justfile
✅ Files skipped from review due to trivial changes (10)
- demo/relay/justfile
- infra/justfile
- .github/justfile
- nix/modules/moq-relay.nix
- .taplo.toml
- nix/overlay.nix
- rs/moq-native/Cargo.toml
- demo/web/justfile
- swift/justfile
- py/justfile
🚧 Files skipped from review as they are similar to previous changes (1)
- flake.nix
CodeRabbit flagged three pre-existing demo justfile bugs while reviewing the lint-tooling PR (they were in the diff only because just --fmt reformatted those files). Small enough to fix in the same PR: - demo/boy/justfile sync: `for file in "$dir"/*` iterates the literal glob when the dir is empty, causing a failing wrangler upload. Skip non-existent paths with `[ -e "$file" ] || continue`. - demo/pub/justfile sync: same fix. - demo/pub/justfile download: `curl -o media/X.mp4` fails on first run if media/ doesn't exist. `mkdir -p media` before downloading. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The repo-wide nixfmt check (top-level justfile, line 50) started failing on every PR because this file drifted from the formatter's expected shape since #1519 wired nixfmt into CI. Run nixfmt over it: 73 lines reflowed, no semantic changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Wires a suite of linters / formatters into the existing
just check/just cipipeline, closing the major unlinted surfaces in the repo:shellcheck+shfmt.github/scripts,go/,kt/,swift/,infra/{apt,rpm},rs/{libmoq,moq-ffi,moq-gst,scripts},demo/throttle/enable)actionlint.github/workflows/*.yml.github/justfilemodule (mod gh '.github')taplo.taplo.tomlnixfmt(RFC-style)flake.nix,nix/overlay.nix,nix/modules/moq-relay.nixjust --fmt --unstablejustfilein-tree (17 total)All tools come from
nixpkgsvia a singlelintDepsgroup inflake.nix, sonix developprovides them automatically andjust cican require them.just check/just fixguard each tool withcommand -vso they skip silently when missing, keeping the inner loop usable outside Nix.What's in the diff
Tooling
flake.nix—lintDeps = [ shellcheck shfmt actionlint taplo nixfmt-rfc-style ], added to the devShellpackages..github/justfile(new) —check(silent skip) andci(required) recipes foractionlint..taplo.toml(new) —indent_string = " ",column_width = 150,align_comments = false, plus anexcludearray fornode_modules,target,dist,.venv.justfile— registersmod gh '.github'; inlines guarded calls to all the new tools incheck/fix(matching the existingbun remarkpattern) and unguarded calls in the always-run block ofci..editorconfig—[*.sh]+[demo/throttle/enable]blocks lock in 4-space indent +switch_case_indentfor shfmt;[{node_modules,target,dist,.venv}/**] ignore = trueso shfmt's recursive walk skips vendored / build output.rs/justfile—cargo sort --workspace --check --no-formatand matching fix, so cargo-sort owns dep ordering and taplo owns whitespace. Without this they fought overquinn = { ..., features = [...] }inrs/moq-native/Cargo.toml.Format-pass fallout (folded in)
All mechanical:
shfmt --write. Mechanical: case bodies with;-separated statements expanded onto multiple lines;\line continuations removed where|already implies continuation.just --fmt.flake.nix(with pkgs; [...]blocks expanded to a let-style), plus light reflows innix/overlay.nixandnix/modules/moq-relay.nix.rs/moq-native/Cargo.tomlcollapsed inline (now undercolumn_width = 150) and one was rewrapped multi-line.Real findings addressed (not just formatting)
SCRIPT_DIRassignment inswift/scripts/publish.sh— removed.run:blocks) flagged six lines across six workflows:docker.yml— quoted"$GITHUB_OUTPUT"and added a targeted# shellcheck disable=SC2046on the intentional word-splittingprintf ... *of digests.release-brew.yml— refactored three consecutiveecho "k=v" >> "$GITHUB_OUTPUT"lines into a single{...} >> "$GITHUB_OUTPUT"block (SC2129).moq-cli.yml,moq-gst.yml,moq-relay.yml,moq-token-cli.yml— added# shellcheck disable=SC2016on theenvsubst '$VAR1 $VAR2 ...'calls (single-quoting the varlist is the correct invocation).Reviewer notes
flake.nix,justfile,.taplo.toml,.editorconfig, and.github/justfile.command -vguard pattern is documented in code comments so future tools can follow the same "optional locally, required in ci" convention.hadolint(1 Dockerfile, 44 lines) — out of scope per discussion.Test plan
nix develop --command just checkpasses locally.nix develop --command just fixis idempotent on the post-pass tree.command -vguards exit 0 silently when a tool is missing from$PATH.check.ymlworkflow goes green on this PR.(Written by Claude)