fix(scripts): apply prettier --write to benchmark-sandbox-image-build.js#1637
fix(scripts): apply prettier --write to benchmark-sandbox-image-build.js#1637cv merged 1 commit intoNVIDIA:mainfrom
Conversation
`scripts/benchmark-sandbox-image-build.js` had line-length drift between the source and the project's prettier config. `npx prettier --check` would warn on it, and the prek pre-push hook auto-fixed it on every contributor's machine — which then triggered the framework's "Files were modified by following hooks" failure path, blocking the push for any contributor whose own diff was unrelated. This is the residual fix for NVIDIA#1636 after NVIDIA#1634 already cleaned up the ESLint and prettier drift in `bin/lib/onboard.js` and `test/onboard.test.js`. The benchmark script wasn't covered by NVIDIA#1634's sweep, so it's still the last file failing `prettier --check` on stock `main`. `prettier --write` produced a handful of `execFileSync(...)` and `run(...)` argument wraps. No semantic change — same calls, same args, same control flow, just line-broken to comply with the configured max width. Refs NVIDIA#1636. Signed-off-by: T Savo <evilgenius@nefariousplan.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR resolves the last remaining Prettier formatting drift reported in #1636 by applying prettier --write to the benchmark script, bringing it in line with the repository’s configured max line width without changing runtime behavior.
Changes:
- Wraps long
run(...)/execFileSync(...)invocations to satisfy Prettier line-length rules. - Reformats multi-argument calls (including
dockerBuild(...)) for consistent indentation and trailing commas.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
LGTM — security review PASS.
Formatting-only change (Prettier line-width wrapping). No logic, control flow, or dependency changes. CI failure is an unrelated fork permission issue (Cleanup artifacts job).
No concerns.
Summary
Residual fix for #1636 after #1634 cleaned up the ESLint and prettier drift in
bin/lib/onboard.jsandtest/onboard.test.js. The benchmark script wasn't covered by that sweep, so it's the last file failingprettier --checkon stockmain.Related Issue
Refs #1636 (#1634 already handled the other two findings — this closes out the residual).
Changes
scripts/benchmark-sandbox-image-build.js: handful ofexecFileSync(...)andrun(...)argument wraps. No semantic change — same calls, same args, same control flow, just line-broken to comply with the configured prettier max width.Diff: +26 / −6 in 1 file.
Type of Change
Testing
npx prettier --check scripts/benchmark-sandbox-image-build.jscleannpx prettier --check bin/lib/onboard.js test/onboard.test.js scripts/benchmark-sandbox-image-build.js— all 3 files now clean (the first two were fixed by fix(lint): resolve ESLint errors introduced by #1564 #1634)node --check scripts/benchmark-sandbox-image-build.jscleanprettier-js,shfmtno longer surfaces any auto-fix in this file (verified by amending an unrelated branch and re-pushing on a fresh clone)Checklist
General
Code Changes
Doc Changes
Signed-off-by: T Savo evilgenius@nefariousplan.com
Summary by CodeRabbit