Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Nov 20, 2025

Summary by CodeRabbit

  • Documentation
    • Updated help text for the reset migrate subcommand to clarify that a seed script defined in package.json will be executed after migration by default, unless the --skip-seed option is specified.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings November 20, 2025 03:08
@coderabbitai
Copy link

coderabbitai bot commented Nov 20, 2025

Walkthrough

Adds help text to the reset migrate subcommand in the CLI to inform users that a seed script defined in package.json will be run after reset, unless the --skip-seed flag is used.

Changes

Cohort / File(s) Change Summary
CLI Help Text
packages/cli/src/index.ts
Extends reset migrate subcommand help output via .addHelpText('after', ...) to document seed script execution behavior and --skip-seed option

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Poem

🐰 A helpful hint now graces the CLI,
When reset runs, the seed script flies free,
Unless you say "skip," and then it won't be—
Our docs grow clearer, hop-pip-hooray! 📝

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore(cli): improve help text of migrate reset' directly and specifically describes the main change in the PR - improving help text documentation for the migrate reset CLI subcommand.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/improve-migrate-reset-cli

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Nov 20, 2025

Claude finished @ymc9's task —— View job


PR Review: Improve help text of migrate reset

Review Checklist:

  • Read and analyze the changed file
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Evaluate security concerns
  • Consider test coverage implications
  • Provide comprehensive feedback

Review Summary

This PR adds helpful clarification text to the migrate reset command in the CLI. The change improves user experience by explaining the seeding behavior.

Detailed Analysis

Code Quality & Best Practices

  • Good: The implementation follows Commander.js patterns correctly using .addHelpText('after', ...)
  • Good: Consistent with existing code style in the file (see db seed command at lines 144-156 which uses the same pattern)
  • Good: The help text is clear and concise, explaining when seeding occurs and how to skip it
  • Good: Proper indentation and formatting matches the rest of the codebase

No Bugs or Issues Detected

  • The syntax is correct and follows Commander.js API properly
  • Text formatting looks appropriate with the leading newline for spacing
  • References the correct option name (--skip-seed) that already exists

Performance Considerations

  • Excellent: This is a static string addition with zero runtime performance impact
  • Help text is only generated when --help is used, so no overhead during normal command execution

Security Concerns

  • None: This is purely documentation/help text with no security implications
  • No user input is processed or executed

📋 Test Coverage Considerations

The existing test in packages/cli/test/migrate.test.ts:20-26 covers the migrate reset --force functionality but doesn't verify help text. Consider adding a test like:

it('should display help text for migrate reset', () => {
    const output = runCliCapture('migrate reset --help', workDir);
    expect(output).toContain('If there is a seed script defined in package.json');
    expect(output).toContain('Use --skip-seed to skip it');
});

However, this is not required as help text testing is often considered lower priority than functional testing.

Minor Suggestions

  1. Consistency: The help text mentions "package.json" but doesn't specify the exact field. The db seed command (lines 147-153) provides a concrete example with "zenstack.seed". Consider whether this level of detail would be helpful here too.

  2. Grammar: Consider adding "the" before "reset" for slightly better readability:

    "it will be run after the reset" → "it will be run after the reset"
    

    (Current text is fine, but this would be slightly more natural)

Overall Assessment

✅ APPROVED - This is a clean, straightforward improvement that enhances user experience without any negative impacts. The change is well-implemented and follows existing patterns in the codebase.

The addition provides valuable context to users about the seeding behavior after reset operations, which is exactly the kind of helpful documentation that improves CLI usability.


Copilot finished reviewing on behalf of ymc9 November 20, 2025 03:09
Copy link
Contributor

Copilot AI left a 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 enhances the user experience by adding contextual help text to the migrate reset command, clarifying the default seeding behavior and how to skip it.

  • Adds .addHelpText() call to the migrate reset command to provide additional usage information

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/cli/src/index.ts (1)

97-100: Help text update is clear and correctly wired to migrate reset.

The addHelpText('after', ...) usage is correct for Commander, and the message accurately reflects the --skip-seed flag behavior. Optionally, you might reference the zenstack.seed config (as in the db seed help text) for consistency, but the current wording is already serviceable.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b4511b and be9ccd1.

📒 Files selected for processing (1)
  • packages/cli/src/index.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: claude-review
  • GitHub Check: build-test (20.x, postgresql)
  • GitHub Check: build-test (20.x, sqlite)

@ymc9 ymc9 merged commit f5c8831 into dev Nov 20, 2025
14 of 15 checks passed
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