-
Notifications
You must be signed in to change notification settings - Fork 663
feat(cli): add --dry-run to run #1308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@claude please take a look |
|
@vrcprl Please take a look |
The-Best-Codes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested this locally yet but looks like a good PR to me!
However, don't forget to run pnpm run format to format and pnpm new to add a changeset before this PR can be merged!
yeah I have run the all tests and builds |
|
@omsherikar You need to run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a --dry-run flag to the run command, allowing users to preview planned translation tasks without executing them or writing files. The feature skips provider initialization and authentication steps, then displays target paths and locales for review.
- Added
--dry-runCLI option that previews tasks without writing files - Modified setup phase to conditionally skip provider/auth steps during dry-run
- Implemented
renderDryRunfunction to display planned tasks with source/target locale information
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/cli/src/cli/utils/ui.ts | Added renderDryRun function to display dry-run preview output |
| packages/cli/src/cli/cmd/run/setup.ts | Modified provider/auth task steps to skip execution when dry-run flag is enabled |
| packages/cli/src/cli/cmd/run/index.ts | Added --dry-run option definition and early return logic after planning phase |
| packages/cli/src/cli/cmd/run/_types.ts | Added dryRun boolean field to flags schema |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (ctx.flags.dryRun) { | ||
| await renderDryRun(ctx.tasks); | ||
| await trackEvent(authId, "cmd.run.success", { | ||
| config: ctx.config, | ||
| flags: ctx.flags, | ||
| }); | ||
| return; | ||
| } |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The frozen function runs before the dry-run check (line 158), but dry-run mode skips provider initialization which may be required by the frozen validation. Consider either: 1) moving the dry-run check before the frozen(ctx) call, or 2) making the frozen function also skip when dryRun is enabled to avoid potential null reference errors when accessing ctx.localizer.
|
Hey @omsherikar! Just checking in - are you still working on this PR? We noticed there are some comments that may need addressing. If you need more time, no problem! Just let us know. If we don't hear back within a week, we'll close this to keep the repo tidy, but you can always reopen when ready. |
Description
Add --dry-run to run to preview planned tasks without writing files/checksums.
Skips provider/auth steps; prints target paths and locales.
Usage: node ./bin/cli.mjs run -y --dry-run [--bucket json] [--target-locale es]
solves #1302