The benchmark scripts are unquoted, and nothing runs shellcheck on them
- Ngôn ngữ chính
- Rust
- Star
- 407
- Fork
- 12
- Merge trung bình
- 5 giờ 45 phút
- Pull request đã merge (30 ngày)
- 30
Mô tả
Split out of #396, which fixed the bugs that stopped `scripts/acceptance` from
working at all. What was left of it has since narrowed, and this is the current
state.
## Done: the BSD-only `sed -i ''`
`scripts/acceptance/test.sh` no longer edits in place. #396 replaced it with a
read-one-name, write-another form that behaves the same on BSD and GNU sed, and
said why in a comment beside it. The `> /dev/null` those lines carried went with
them. Nothing in `scripts/` runs `sed -i` today, and the macOS-only claim this
issue was opened under no longer holds.
## Left: the benchmark scripts are unquoted
`shellcheck` 0.10.0 reports ten findings across `scripts/`, all of them in
`scripts/benchmarks`. `scripts/acceptance` is clean — #396 rewrote it.
```
scripts/benchmarks/setup.sh:3:14: warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. [SC2164]
scripts/benchmarks/setup.sh:3:17: warning: Quote this to prevent word splitting. [SC2046]
scripts/benchmarks/setup.sh:3:27: note: Double quote to prevent globbing and word splitting. [SC2086]
scripts/benchmarks/setup.sh:8:1: warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. [SC2164]
scripts/benchmarks/setup.sh:8:4: note: Double quote to prevent globbing and word splitting. [SC2086]
scripts/benchmarks/setup.sh:10:1: warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. [SC2164]
scripts/benchmarks/comparison.sh:146:11: note: Expressions don't expand in single quotes [SC2016]
scripts/benchmarks/comparison.sh:147:10: note: Expressions don't expand in single quotes [SC2016]
scripts/benchmarks/comparison.sh:148:19: note: Expressions don't expand in single quotes [SC2016]
scripts/benchmarks/comparison.sh:150:3: note: Expressions don't expand in single quotes [SC2016]
```
The two files are not the same job.
- **`comparison.sh`'s four are deliberate.** Those single-quoted strings are
what `hyperfine` labels its results with, and expanding them would print the
same label for every run — the comment above them says so. They want a
`# shellcheck disable=SC2016` with that reason attached, not a change.
- **`setup.sh`'s six are real**, and they are on the three lines #399 is about:
`cd $DATA_ROOT` and `cd gitlab` with nothing to stop the script when the
directory is not there, which is how `git sparse-checkout set doc` reaches
mado's own checkout and empties it. Quoting alone does not fix that — #399
needs a shape, not a `|| exit` — but the same lines are being rewritten there.
Do #399 first and this becomes a re-run of `shellcheck` over the result.
## And: nothing runs it
No workflow lints a shell script. Which linter, which version, and how it is
run is #397, which is also deciding it for `action/entrypoint.sh` — seven
findings there, and outside this issue. Whatever that settles should enforce
`scripts/` too, and pin its version: the runner image ships 0.9.0 where local is
0.10.0, and a linter that is not pinned is a gate that changes under the tree.
Hướng dẫn đóng góp
Đánh giá
Issue này chưa được đánh giá.