feat(cli): add wgc grpc-service list-templates & init#2033
Conversation
jensneuse
requested review from
a team,
JivusAyrus,
StarpTech and
endigma
as code owners
|
Warning Rate limit exceeded@jensneuse has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 21 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes introduce two new CLI commands under Changes
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Router image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
cli/src/commands/grpc-service/commands/init.ts (1)
33-54: Consider extracting template validation logic.The template existence check and error handling could be extracted into a reusable function, especially since it shares logic with the list-templates command.
Consider creating a shared utility module for GitHub API interactions to reduce code duplication between
init.tsandlist-templates.ts. This would centralize error handling and type safety improvements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
cli/package.json(3 hunks)cli/src/commands/grpc-service/commands/init.ts(1 hunks)cli/src/commands/grpc-service/commands/list-templates.ts(1 hunks)cli/src/commands/grpc-service/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
cli/src/commands/grpc-service/commands/init.ts (3)
cli/src/commands/grpc-service/index.ts (1)
opts(7-15)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)cli/src/commands/grpc-service/commands/list-templates.ts (1)
fetchAvailableTemplates(7-16)
cli/src/commands/grpc-service/commands/list-templates.ts (1)
cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)
⏰ 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). (14)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: image_scan
- GitHub Check: integration_test (./events)
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: build_push_image
- GitHub Check: build_test
🔇 Additional comments (2)
cli/package.json (1)
57-57: Dependency Version and Vulnerability Check Completed
- degit@^2.8.4 is the latest stable release (versions 2.8.0–2.8.4).
- node-fetch@^3.3.2 is the latest stable release (next tags are all 4.0.0-beta).
- @types/degit@^2.8.6 is the latest release.
- No known vulnerabilities were reported for these packages in the npm registry.
To perform a full audit, add a lockfile (npm install --package-lock-only) and rerunnpm audit.Files reviewed:
- cli/package.json (lines 57, 69, 88)
cli/src/commands/grpc-service/index.ts (1)
4-5: LGTM! Clean command registration.The new commands are properly imported and registered following the existing pattern used for
generateCommand.Also applies to: 11-12
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
cli/src/commands/grpc-service/commands/init.ts (2)
14-22: Improve type safety and error handling for template validation.This function still has the same issues mentioned in previous reviews: no error handling, no timeout, and poor type safety.
104-115: Potential race condition in directory validation.This code has the same race condition issue mentioned in previous reviews between checking directory existence and creating/validating it.
🧹 Nitpick comments (1)
cli/src/commands/grpc-service/commands/init.ts (1)
24-24: Improve parameter type safety.The
spinnerparameter should be properly typed instead of usingany.-async function downloadAndExtractTemplate(template: string, outputDir: string, spinner: any) { +async function downloadAndExtractTemplate(template: string, outputDir: string, spinner: ReturnType<typeof Spinner>) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
cli/package.json(5 hunks)cli/src/commands/grpc-service/commands/init.ts(1 hunks)cli/src/commands/grpc-service/commands/list-templates.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cli/package.json
- cli/src/commands/grpc-service/commands/list-templates.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
cli/src/commands/grpc-service/commands/init.ts (3)
cli/src/commands/grpc-service/index.ts (1)
opts(7-15)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)cli/src/commands/grpc-service/commands/list-templates.ts (1)
fetchAvailableTemplates(7-22)
⏰ 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). (14)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./events)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: image_scan
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
|
Psychotic git tooling my bad |
|
LGTM after addressing the comments about error handling. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cli/src/commands/grpc-service/commands/generate.ts (1)
41-44: Approve the enhanced directory handling with a minor considerationThe change from failing immediately to automatically creating the output directory is a significant UX improvement. The logic is sound and the
recursive: trueoption handles nested directory creation properly.However, consider the potential race condition between
existsSync()andmkdir(). While unlikely in typical CLI usage, the current implementation could theoretically fail if another process creates the directory between the check and creation attempt.Consider this more robust approach that eliminates the race condition:
- // Ensure output directory exists - if (!existsSync(options.output)) { - await mkdir(options.output, { recursive: true }); - } + // Ensure output directory exists + await mkdir(options.output, { recursive: true });The
recursive: trueoption withmkdiris idempotent and won't throw an error if the directory already exists, making theexistsSynccheck unnecessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cli/src/commands/grpc-service/commands/generate.ts(2 hunks)cli/src/commands/grpc-service/commands/init.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/src/commands/grpc-service/commands/init.ts
⏰ 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). (14)
- GitHub Check: integration_test (./events)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./telemetry)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: image_scan
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_test
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
cli/src/commands/grpc-service/commands/generate.ts (1)
1-1: LGTM: Clean import optimizationThe import changes correctly add the required
mkdirfunction while removing the unuseddirnameimport. This aligns with the new directory creation functionality.Also applies to: 3-3
Co-authored-by: endigma <endigma@mailcat.ca>
Co-authored-by: endigma <endigma@mailcat.ca>
3 participants
Summary by CodeRabbit
New Features
Chores