Skip to content

Conversation

@Joeavaikath
Copy link
Contributor

Why the changes were made

  • Adds ldflags: oadp version will now display the version number and commit hash
  • Adds example text replacement: velero mentions are now replaced with oadp when used in example commands

How to test the changes made

Build and run any command to see the changes

Signed-off-by: Joseph <jvaikath@redhat.com>
Signed-off-by: Joseph <jvaikath@redhat.com>
Copy link
Member

@shubham-pampattiwar shubham-pampattiwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this! The regex approach is clean and the tests look solid.

Question about the Short/Long descriptions

I noticed the help output still shows "Velero" in the command descriptions:

$ ./kubectl-oadp --help
Available Commands:
  bug               Report a Velero bug
  client            Velero client related commands
  create            Create velero resources
  version           Print the velero version and associated image

Was this intentional or should these also say "OADP"?

I can see it both ways:

Keeping "Velero":

  • Makes sense if we're thinking of oadp as a CLI wrapper around Velero
  • Like how kubectl is the CLI but you're still managing "Kubernetes"
  • "Report a Velero bug" = bug in the Velero project
  • "Create velero resources" = Velero CRDs

Changing to "OADP":

  • Better consistency - users only see "OADP" everywhere
  • Simplifies the mental model
  • They installed oadp-cli, not velero-cli

Right now your PR fixes:

  • ✅ Examples in help text
  • ✅ Runtime messages like Run \velero backup describe``
  • ❓ Command descriptions (Short/Long fields)

What's your take - should we update those descriptions too, or leave them referencing Velero?

Signed-off-by: Joseph <jvaikath@redhat.com>
@Joeavaikath
Copy link
Contributor Author

@shubham-pampattiwar
We want the CLI user to use the right command (oadp)
The entities being created are velero resources though, so keeping them makes sense in my mind

@shubham-pampattiwar
Copy link
Member

yeah thats what I was inclined towards too, Thank you ! @Joeavaikath

Signed-off-by: Joseph <jvaikath@redhat.com>
@shubham-pampattiwar
Copy link
Member

/lgtm

@Joeavaikath Joeavaikath merged commit aed843d into migtools:oadp-dev Nov 19, 2025
15 checks passed
Copy link
Member

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace upstream command.Example texts from velero <command> <verb> to oadp <command> <verb>

3 participants