ci: authenticate nksdeploy phar clone#50
Conversation
📝 WalkthroughWalkthroughThe script adds a ChangesGitHub Token Authentication for Git Clone
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/build-nksdeploy-phar.mjs (1)
145-148: ⚡ Quick winPrefer argv-based Git execution over shell quoting.
This still depends on shell parsing on every platform. Since
spawnSyncis already available in this file, callinggitwith an argv array would be more portable and would let you dropshellQuote()from the clone path entirely.♻️ Proposed refactor
- execSync(`git clone --depth 1 https://github.com/nks-hub/nksdeploy ${shellQuote(tmp)}`, { - stdio: 'inherit', - env: cloneEnv, - }) + const clone = spawnSync('git', ['clone', '--depth', '1', 'https://github.com/nks-hub/nksdeploy', tmp], { + stdio: 'inherit', + env: cloneEnv, + }) + if (clone.status !== 0) { + die(`git clone failed with exit ${clone.status}`) + }🤖 Prompt for 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. In `@scripts/build-nksdeploy-phar.mjs` around lines 145 - 148, Replace the execSync call that uses shellQuote(tmp) with a spawnSync invocation to run git with argv style; specifically, stop using execSync(...) and shellQuote and instead call spawnSync('git', ['clone', '--depth', '1', 'https://github.com/nks-hub/nksdeploy', tmp], { stdio: 'inherit', env: cloneEnv }) (use the existing spawnSync import), and preserve error handling/exit behavior if the spawnSync result indicates failure; remove the shellQuote(tmp) usage entirely.
🤖 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.
Nitpick comments:
In `@scripts/build-nksdeploy-phar.mjs`:
- Around line 145-148: Replace the execSync call that uses shellQuote(tmp) with
a spawnSync invocation to run git with argv style; specifically, stop using
execSync(...) and shellQuote and instead call spawnSync('git', ['clone',
'--depth', '1', 'https://github.com/nks-hub/nksdeploy', tmp], { stdio:
'inherit', env: cloneEnv }) (use the existing spawnSync import), and preserve
error handling/exit behavior if the spawnSync result indicates failure; remove
the shellQuote(tmp) usage entirely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f71957c-c428-49f8-9114-5d792d640f08
📒 Files selected for processing (1)
scripts/build-nksdeploy-phar.mjs
Summary
Verification
Summary by CodeRabbit