Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the project from urfave/cli/v2 to v3, following the official migration guide. The changes update all CLI-related code to use the new v3 API patterns.
Key Changes:
- Updated import paths from
github.com/urfave/cli/v2togithub.com/urfave/cli/v3 - Changed
EnvVarsfield toSources: cli.EnvVars()for all flags - Updated action function signatures to accept
context.Contextand*cli.Commandparameters - Modified application initialization to use
cli.Commandinstead ofcli.App
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/config/flags.go | Updated all flag definitions to use Sources instead of EnvVars |
| go.mod | Updated dependency from cli/v2 to cli/v3 and removed obsolete transitive dependencies |
| cmd/tls/main.go | Updated CLI app initialization and action signatures to v3 patterns |
| cmd/cli/*.go | Updated all CLI command action functions to use new v3 signature |
| cmd/api/main.go | Updated API service CLI initialization and action handlers |
| cmd/admin/main.go | Updated admin service CLI initialization and action handlers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cmd/cli/main.go
Outdated
| if len(cmd.Flags) == 0 { | ||
| if err := cli.ShowCommandHelp(ctx, cmd, cmd.Name); err != nil { | ||
| log.Fatal().Msgf("❌ Error with CLI help - %s", err) | ||
| } | ||
| return cli.Exit("\nNo flags provided", 2) |
There was a problem hiding this comment.
The condition len(cmd.Flags) == 0 will always be false since flags are appended in the Command initialization. The original v2 code used c.NumFlags() which checked for provided flags. Consider using cmd.NArg() == 0 && !cmd.Bool(\"help\") or a similar pattern to check if no arguments/flags were actually provided by the user.
| if len(cmd.Flags) == 0 { | |
| if err := cli.ShowCommandHelp(ctx, cmd, cmd.Name); err != nil { | |
| log.Fatal().Msgf("❌ Error with CLI help - %s", err) | |
| } | |
| return cli.Exit("\nNo flags provided", 2) | |
| if cmd.NArg() == 0 && !cmd.Bool("help") { | |
| if err := cli.ShowCommandHelp(ctx, cmd, cmd.Name); err != nil { | |
| log.Fatal().Msgf("❌ Error with CLI help - %s", err) | |
| } | |
| return cli.Exit("\nNo flags or arguments provided", 2) |
cmd/tls/main.go
Outdated
| { | ||
| Name: "help", | ||
| Action: func(ctx context.Context, cmd *cli.Command) error { | ||
| cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0) |
There was a problem hiding this comment.
Using cmd.Name as the command name parameter is incorrect for the help command. This should be the parent command's name or empty string to show app-level help. Change to cli.ShowCommandHelpAndExit(ctx, cmd.Parent, \"\", 0) or similar.
| cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0) | |
| cli.ShowCommandHelpAndExit(ctx, cmd, "", 0) |
cmd/api/main.go
Outdated
| { | ||
| Name: "help", | ||
| Action: func(ctx context.Context, cmd *cli.Command) error { | ||
| cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0) |
There was a problem hiding this comment.
Using cmd.Name (which is "help") as the command name parameter is incorrect. This should show the app-level help, not help for the help command itself. Change to cli.ShowCommandHelpAndExit(ctx, cmd.Parent, \"\", 0) or use an empty string for the command name.
| cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0) | |
| cli.ShowCommandHelpAndExit(ctx, cmd, "", 0) |
cmd/admin/main.go
Outdated
| { | ||
| Name: "help", | ||
| Action: func(ctx context.Context, cmd *cli.Command) error { | ||
| cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0) |
There was a problem hiding this comment.
Using cmd.Name (which is "help") as the command name parameter is incorrect. This should show the app-level help. Change to cli.ShowCommandHelpAndExit(ctx, cmd.Parent, \"\", 0) or use an empty string for the command name.
| cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0) | |
| cli.ShowCommandHelpAndExit(ctx, cmd.Parent, "", 0) |
Migrating
urfave/clitov3as described here https://cli.urfave.org/migrate-v2-to-v3/