[commands] wrap server into CLI#14
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughApplication startup is refactored to use urfave/cli. The FX initialization and lifecycle are moved into a new Changes
Sequence Diagram(s)sequenceDiagram
actor OS
participant Main as Main
participant CLI as "urfave/cli"
participant Context as Context
participant Serve as Serve()
participant FX as "Uber FX App"
participant Logger as "*zap.Logger"
OS->>Main: Run(version)
Main->>CLI: create CLI app + backend serve command
Main->>Context: create cancellable context
OS->>Main: register SIGINT/SIGTERM -> cancel Context
CLI->>Serve: invoke serve action
Serve->>FX: construct FX graph (modules + version)
FX->>Logger: onStart hook -> log "app started"
FX->>FX: Start with timeout
Note over Context,FX: wait for Context cancel or FX done
OS->>Context: signal -> cancel
FX->>Logger: onStop hook -> log "app stopped"
FX->>FX: Stop with timeout
FX-->>Serve: return result
Serve-->>CLI: return error/status
CLI-->>Main: exit with code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
🤖 Pull request artifacts
|
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 `@internal/commands/serve.go`:
- Around line 28-29: The Serve function currently starts the FX app with
fx.App.Run(), which prevents the outer CLI context from cancelling the app;
change Serve to accept a context.Context and return an error, instantiate the
fx.App as you do now but replace Run() with app.Start(ctx), then wait for
app.Done() (or app.Wait()) while monitoring ctx, and finally call app.Stop(ctx)
on cancellation or completion; update the caller in internal/app.go to call and
return commands.Serve(ctx, version) so the CLI context is propagated.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b0ce73f-f50a-4778-acf3-7bbd0f03f8fb
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.modinternal/app.gointernal/commands/serve.go
Co-authored-by: Copilot <copilot@github.com>
64d2774 to
536ccaa
Compare
Summary by CodeRabbit
Chores
Refactor
New Features