Skip to content

Conversation

@javuto
Copy link
Collaborator

@javuto javuto commented Nov 25, 2025

Migrating urfave/cli to v3 as described here https://cli.urfave.org/migrate-v2-to-v3/

@javuto javuto requested a review from Copilot November 25, 2025 12:00
@javuto javuto added refactor Refactorization of code osctrl-tls osctrl-tls related changes osctrl-api osctrl-api related changes osctrl-admin osctrl-admin related changes osctrl-cli osctrl-cli related changes labels Nov 25, 2025
Copy link
Contributor

Copilot AI left a 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/v2 to github.com/urfave/cli/v3
  • Changed EnvVars field to Sources: cli.EnvVars() for all flags
  • Updated action function signatures to accept context.Context and *cli.Command parameters
  • Modified application initialization to use cli.Command instead of cli.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.

Comment on lines +2114 to 2118
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)
Copy link

Copilot AI Nov 25, 2025

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.

Suggested change
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)

Copilot uses AI. Check for mistakes.
{
Name: "help",
Action: func(ctx context.Context, cmd *cli.Command) error {
cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0)
Copy link

Copilot AI Nov 25, 2025

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.

Suggested change
cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0)
cli.ShowCommandHelpAndExit(ctx, cmd, "", 0)

Copilot uses AI. Check for mistakes.
{
Name: "help",
Action: func(ctx context.Context, cmd *cli.Command) error {
cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0)
Copy link

Copilot AI Nov 25, 2025

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.

Suggested change
cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0)
cli.ShowCommandHelpAndExit(ctx, cmd, "", 0)

Copilot uses AI. Check for mistakes.
{
Name: "help",
Action: func(ctx context.Context, cmd *cli.Command) error {
cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0)
Copy link

Copilot AI Nov 25, 2025

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.

Suggested change
cli.ShowCommandHelpAndExit(ctx, cmd, cmd.Name, 0)
cli.ShowCommandHelpAndExit(ctx, cmd.Parent, "", 0)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

osctrl-admin osctrl-admin related changes osctrl-api osctrl-api related changes osctrl-cli osctrl-cli related changes osctrl-tls osctrl-tls related changes refactor Refactorization of code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants