fix(lint): resolve ESLint errors introduced by #1564#1634
Conversation
- test/onboard.test.js: replace __dirname with import.meta.dirname (test files are ESM, __dirname is CJS-only) - bin/lib/onboard.js: remove @typescript-eslint/no-require-imports disable comment (rule doesn't exist in CJS ESLint config, require is valid in CJS) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoved an inline ESLint disable in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ericksoa
left a comment
There was a problem hiding this comment.
LGTM — fixes broken main build. ESM __dirname → import.meta.dirname, removes stale eslint-disable comment, Prettier reformatting.
`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>
….js (#1637) <!-- markdownlint-disable MD041 --> ## Summary Residual fix for #1636 after #1634 cleaned up the ESLint and prettier drift in `bin/lib/onboard.js` and `test/onboard.test.js`. The benchmark script wasn't covered by that sweep, so it's the last file failing `prettier --check` on stock `main`. ## Related Issue Refs #1636 (#1634 already handled the other two findings — this closes out the residual). ## Changes - `scripts/benchmark-sandbox-image-build.js`: 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 prettier max width. Diff: +26 / −6 in 1 file. ## Type of Change - [x] Code change for a new feature, bug fix, or refactor. - [ ] Code change with doc updates. - [ ] Doc only. Prose changes without code sample modifications. - [ ] Doc only. Includes code sample changes. ## Testing - [x] `npx prettier --check scripts/benchmark-sandbox-image-build.js` clean - [x] `npx 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 #1634) - [x] `node --check scripts/benchmark-sandbox-image-build.js` clean - [x] Re-running the prek pre-push gate with `prettier-js,shfmt` no longer surfaces any auto-fix in this file (verified by amending an unrelated branch and re-pushing on a fresh clone) - [ ] Upstream CI (will run on PR open) ## Checklist ### General - [x] I have read and followed the [contributing guide](https://github.com/NVIDIA/NemoClaw/blob/main/CONTRIBUTING.md). - [ ] I have read and followed the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md). (for doc-only changes — N/A) ### Code Changes - [x] Formatters applied — this PR *is* the formatter output. - [x] Tests added or updated for new or changed behavior — N/A, formatting-only. - [x] No secrets, API keys, or credentials committed. - [ ] Doc pages updated for any user-facing behavior changes — N/A, no behavior change. ### Doc Changes - N/A (no doc changes) --- Signed-off-by: T Savo <evilgenius@nefariousplan.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Internal script formatting improvements with no functional changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: T Savo <evilgenius@nefariousplan.com>
Summary
Fixes the broken
mainbuild caused by two ESLint errors from #1564:test/onboard.test.js:597—__dirnameis not defined in ESM. Replaced withimport.meta.dirname.bin/lib/onboard.js:424—@typescript-eslint/no-require-importsrule doesn't exist in the CJS ESLint config. Removed the unnecessary disable comment (requireis valid in CJS).Test plan
checksjob passes (ESLint CLI step)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores
Tests