fix(migrate): summarize skipped git hooks#1864
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc46f81c23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (plan.gitHooksSkipReason) { | ||
| addManualStep(report, `Git hooks were not migrated: ${plan.gitHooksSkipReason}`); |
There was a problem hiding this comment.
collectMigrationSetupPlan is shared by the existing-Vite+ branch, but this is the only place that turns gitHooksSkipReason into a manual step, and that branch later prints a summary only for didMigrate || report.warnings.length > 0. In an already-migrated project that still has legacy hooks but preflight skips them (for example an unsupported lint-staged config or another hook tool), the user sees only the inline warning and the final “already using Vite+” path still omits the Manual follow-up this change is meant to add. Add the manual step, and include manual steps in that branch’s summary gate, there as well.
Useful? React with 👍 / 👎.
|
Updated the migration snap fixtures for the skipped git-hook manual follow-up output shown by CI.\n\nValidation:\n- git diff --check\n- Verified the changed files match the 10 failing snap fixtures reported in the CI diff\n\nNote: I tried a targeted local snap-test invocation, but this checkout's snap-test runner rejected the guessed --update/--case options, so I kept validation to the CI-provided golden diff plus whitespace checks. |
|
Discussions regarding this matter are still ongoing, and there are several points that need to be finalized before we modify the code. Specifically, we need to decide whether the next step should be preparing the documentation, improving the CLI UX, or adjusting the migration behavior. |
|
Thank you for your work on this. After the discussion in #1854, the agreed direction is to document the current behavior in the migration guide I’ll take it from here. |
2 participants
Summary
Closes #1854
Verification
git diff --checkpnpm exec tsgo --noEmit -p packages/cli/tsconfig.jsonNotes
pnpm -F vite-plus test src/migration/__tests__/migrator.spec.ts -- --runInBandcould not start in this checkout because Vitest resolves the local Vite workspace package andvite/dist/vite/node/index.jshas not been built. This is the same local workspace-dist blocker seen while preparing #1863.