Skip to content

refactor: extract check command into its own module#1350

Open
fengmk2 wants to merge 2 commits intowindows-installerfrom
refactor-cli.rs
Open

refactor: extract check command into its own module#1350
fengmk2 wants to merge 2 commits intowindows-installerfrom
refactor-cli.rs

Conversation

@fengmk2
Copy link
Copy Markdown
Member

@fengmk2 fengmk2 commented Apr 9, 2026

Move the check command's orchestration logic, output analysis types,
and parsing functions from cli.rs into a dedicated check/ directory,
following the same pattern as exec/.

cli.rs: 1,778 → 1,314 lines
check/mod.rs: execute_check() orchestration (247 lines)
check/analysis.rs: types, parsers, formatters (255 lines)


Note

Medium Risk
Primarily a refactor, but it changes how vp check is dispatched and how fmt/lint output is captured/parsed, which could affect exit codes or CI output stability.

Overview
Refactors the vp check implementation by extracting its orchestration and output-parsing logic from cli.rs into a new check/ module (check/mod.rs + check/analysis.rs), keeping the same fmt→lint flow and --fix re-format pass.

Updates cli.rs visibility to reuse resolve_universal_vite_config and resolve_and_capture_output, and switches the Check subcommand handler to delegate to crate::check::execute_check; related LintMessageKind tests move with the extraction. Documentation in rfcs/check-command.md is updated to reflect the new module layout and cache-input helper naming.

Reviewed by Cursor Bugbot for commit e9cf38e. Configure here.

@fengmk2 fengmk2 self-assigned this Apr 9, 2026
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 9, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label auto-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 9, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ 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".

@fengmk2 fengmk2 force-pushed the windows-installer branch from 7bb82d7 to 2b0a367 Compare April 9, 2026 13:33
@fengmk2 fengmk2 force-pushed the windows-installer branch from 2b0a367 to 2e2720b Compare April 9, 2026 13:41
@fengmk2 fengmk2 force-pushed the refactor-cli.rs branch 2 times, most recently from 5ad7900 to e9cf38e Compare April 10, 2026 02:45
@fengmk2
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 10, 2026

@cursor review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e9cf38e. Configure here.

fengmk2 added 2 commits April 10, 2026 14:00
Move the check command's orchestration logic, output analysis types,
and parsing functions from cli.rs into a dedicated check/ directory,
following the same pattern as exec/.

cli.rs: 1,778 → 1,314 lines
check/mod.rs: execute_check() orchestration (247 lines)
check/analysis.rs: types, parsers, formatters (255 lines)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants