Conversation
kibertoad
requested review from
a team,
CarlosGamero and
dariacm
as code owners
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request updates documentation and CLI behavior for the Estimated code review effort🎯 2 (Simple) | ⏱️ ~9 minutes 🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/drizzle-utils/src/cli/markMigrationsApplied.ts`:
- Around line 112-116: The CLI currently treats the last positional (const
configPath = positionals.at(-1)) as the config path even when npx injects only
the bin name; update the check to treat a single injected positional as
"missing" by verifying positionals.length > 1 before accepting configPath —
i.e., if !configPath or positionals.length === 1 then call printUsage(1).
Reference the existing variables positionals, configPath and the function
printUsage to locate and update the logic in markMigrationsApplied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: lokalise/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1aa3f81-d94f-40de-96e8-e68067c7d10e
📒 Files selected for processing (4)
packages/app/drizzle-utils/README.mdpackages/app/drizzle-utils/docs/migrating-from-prisma.mdpackages/app/drizzle-utils/src/cli/markMigrationsApplied.test.tspackages/app/drizzle-utils/src/cli/markMigrationsApplied.ts
| // When invoked via `npx @lokalise/drizzle-utils mark-migrations-applied ./config.ts`, | ||
| // npx passes the bin name as a positional arg too. Use the last positional to be resilient. | ||
| const configPath = positionals.at(-1) | ||
| if (!configPath) { | ||
| printUsage(1) |
There was a problem hiding this comment.
Treat lone injected bin positional as “missing config path”.
If invocation is npx @lokalise/drizzle-utils mark-migrations-applied (no config path), Line 114 resolves the bin name as configPath, and the CLI later fails with an import error instead of usage output.
💡 Proposed fix
function parseCliArgs(): { configPath: string } {
const { values, positionals } = parseArgs({
@@
if (values.help) {
printUsage(0)
}
+ const normalizedPositionals =
+ positionals[0] === 'mark-migrations-applied' ? positionals.slice(1) : positionals
+
// When invoked via `npx `@lokalise/drizzle-utils` mark-migrations-applied ./config.ts`,
// npx passes the bin name as a positional arg too. Use the last positional to be resilient.
- const configPath = positionals.at(-1)
+ const configPath = normalizedPositionals.at(-1)
if (!configPath) {
printUsage(1)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // When invoked via `npx @lokalise/drizzle-utils mark-migrations-applied ./config.ts`, | |
| // npx passes the bin name as a positional arg too. Use the last positional to be resilient. | |
| const configPath = positionals.at(-1) | |
| if (!configPath) { | |
| printUsage(1) | |
| const normalizedPositionals = | |
| positionals[0] === 'mark-migrations-applied' ? positionals.slice(1) : positionals | |
| // When invoked via `npx `@lokalise/drizzle-utils` mark-migrations-applied ./config.ts`, | |
| // npx passes the bin name as a positional arg too. Use the last positional to be resilient. | |
| const configPath = normalizedPositionals.at(-1) | |
| if (!configPath) { | |
| printUsage(1) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/app/drizzle-utils/src/cli/markMigrationsApplied.ts` around lines 112
- 116, The CLI currently treats the last positional (const configPath =
positionals.at(-1)) as the config path even when npx injects only the bin name;
update the check to treat a single injected positional as "missing" by verifying
positionals.length > 1 before accepting configPath — i.e., if !configPath or
positionals.length === 1 then call printUsage(1). Reference the existing
variables positionals, configPath and the function printUsage to locate and
update the logic in markMigrationsApplied.
2 participants
Changes
correctly handle multi-arg npx invocation
Checklist
major,minor,patchorskip-releaseAI Assistance Tracking
We're running a metric to understand where AI assists our engineering work. Please select exactly one of the options below:
Mark "Yes" if AI helped in any part of this work, for example: generating code, refactoring, debugging support,
explaining something, reviewing an idea, or suggesting an approach.