-
Notifications
You must be signed in to change notification settings - Fork 62
Migrate urfave/cli to v3 #750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 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.
| 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) |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
| { | ||
| Name: "help", | ||
| Action: func(ctx context.Context, cmd *cli.Command) error { | ||
| cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0) |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
| { | ||
| Name: "help", | ||
| Action: func(ctx context.Context, cmd *cli.Command) error { | ||
| cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0) |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
| { | ||
| Name: "help", | ||
| Action: func(ctx context.Context, cmd *cli.Command) error { | ||
| cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0) |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/