chore: add reset main workflow command#69
Conversation
|
@coderabbitai re-review |
📝 WalkthroughWalkthroughThe repository's reset workflow is updated to fetch from the remote main branch before resetting the local working tree to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/reset-main.test.mjs (1)
1-4:⚠️ Potential issue | 🔴 CriticalMissing
testimport causes pipeline failure.The
testfunction is used on lines 13, 30, and 35, but it's never imported. This causes aReferenceErrorand is likely the cause of the CI failure. For Node.js's built-in test runner, you need to importtestfromnode:test.🐛 Proposed fix
import assert from 'assert'; import fs from 'fs'; import path from 'path'; import { fileURLToPath } from 'url'; +import { test } from 'node:test';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/reset-main.test.mjs` around lines 1 - 4, The test suite is calling the global test function but never imports it; add an explicit import for test from Node's test runner (e.g., include "import { test } from 'node:test'" alongside the existing imports) so the references to test() in this file resolve and the CI no longer throws ReferenceError; ensure the import is placed at the top with other imports where functions like test are used.
🧹 Nitpick comments (2)
cmd/reset-main.js (1)
19-43: Consider adding error handling for workflow steps.The
main()function catches errors from the initial repository check but doesn't handle failures in the subsequent git commands (lines 27-40). Ifgit fetchfails due to network issues orgit checkoutfails due to conflicts, the script will throw an uncaught exception with a less informative error message.💡 Proposed improvement
function main() { try { run('git rev-parse --is-inside-work-tree'); } catch (err) { console.error('Not inside a git repository.'); process.exit(1); } + try { console.log('[1/5] Fetch origin/main'); run('git fetch origin main --prune'); console.log('[2/5] Checkout main'); run('git checkout main'); console.log('[3/5] Reset local changes to origin/main'); run('git reset --hard origin/main'); console.log('[4/5] Remove untracked files'); run('git clean -fd'); console.log('[5/5] Final status'); run('git status --short --branch'); console.log('Done. Working tree synced to origin/main.'); + } catch (err) { + console.error('Reset workflow failed:', err.message); + process.exit(1); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/reset-main.js` around lines 19 - 43, The main() function currently only handles errors from the initial git check; wrap each subsequent git step (the run(...) calls for 'git fetch origin main --prune', 'git checkout main', 'git reset --hard origin/main', 'git clean -fd', and 'git status --short --branch') with error handling so failures produce clear messages and exit non‑zero: either put each run call into a try/catch that logs a descriptive error via console.error (including the command and the caught error) and calls process.exit(1), or update the run(...) helper to return a status/error and check it after each invocation; reference the main() function and each run(...) invocation so the change is applied to those exact calls.package.json (1)
29-29: Consider whethercmd/should be added tofilesfor npm publishing.The new
resetscript referencescmd/reset-main.js, but thecmd/directory is not included in thefilesarray (lines 9-19). If this script is intended for development/maintenance only and not for end-users of the published package, this is fine. Otherwise, add"cmd/"to thefilesarray.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 29, The package.json adds a "reset" npm script that runs "cmd/reset-main.js" but the package's "files" array does not include the cmd/ directory; decide whether cmd/reset-main.js should be published and if so add "cmd/" to the "files" array in package.json, otherwise leave it out and document that the script is development-only (or move the script into a published location). Locate the "reset" script string and the "files" array in package.json to make the appropriate update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/unit/reset-main.test.mjs`:
- Around line 1-4: The test suite is calling the global test function but never
imports it; add an explicit import for test from Node's test runner (e.g.,
include "import { test } from 'node:test'" alongside the existing imports) so
the references to test() in this file resolve and the CI no longer throws
ReferenceError; ensure the import is placed at the top with other imports where
functions like test are used.
---
Nitpick comments:
In `@cmd/reset-main.js`:
- Around line 19-43: The main() function currently only handles errors from the
initial git check; wrap each subsequent git step (the run(...) calls for 'git
fetch origin main --prune', 'git checkout main', 'git reset --hard origin/main',
'git clean -fd', and 'git status --short --branch') with error handling so
failures produce clear messages and exit non‑zero: either put each run call into
a try/catch that logs a descriptive error via console.error (including the
command and the caught error) and calls process.exit(1), or update the run(...)
helper to return a status/error and check it after each invocation; reference
the main() function and each run(...) invocation so the change is applied to
those exact calls.
In `@package.json`:
- Line 29: The package.json adds a "reset" npm script that runs
"cmd/reset-main.js" but the package's "files" array does not include the cmd/
directory; decide whether cmd/reset-main.js should be published and if so add
"cmd/" to the "files" array in package.json, otherwise leave it out and document
that the script is development-only (or move the script into a published
location). Locate the "reset" script string and the "files" array in
package.json to make the appropriate update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 79b8cdaf-3d3b-46e0-8162-cda28e5e51a6
📒 Files selected for processing (3)
cmd/reset-main.jspackage.jsontests/unit/reset-main.test.mjs
📜 Review details
🧰 Additional context used
🪛 GitHub Actions: ci
package.json
[error] Command "npm run test:unit && npm run test:e2e" failed because test:unit returned a non-zero exit code (1 test failed).
🔇 Additional comments (3)
cmd/reset-main.js (1)
27-28: LGTM on the fetch-and-reset approach.Using
git fetch origin main --prunefollowed bygit reset --hard origin/mainis a cleaner and more deterministic approach thangit pull, as it avoids potential merge conflicts and ensures the local branch exactly matches the remote.tests/unit/reset-main.test.mjs (2)
13-27: Test assertions for git command sequence are correct.The test properly verifies the new fetch-and-reset workflow and explicitly checks that
git pull origin mainis not present, which aligns with the PR's intent to replace pull-based sync with fetch + hard reset.
35-38: Good: Test validates npm script integration.The test correctly verifies that
package.jsonexposes the reset workflow via theresetscript. The BOM stripping on line 36 is good defensive coding for Windows-originated JSON files.
Summary
Tests
Summary by CodeRabbit
Release Notes
New Features
resetcommand that synchronizes your local main branch with the remote origin to ensure consistency.Chores