Skip to content
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

chore: Get executable name from the command #326

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

dwertent
Copy link
Contributor

This PR enhances the FireFly CLI by making the executable name dynamic in the help messages. Instead of hardcoding the executable name, it is now loaded from the command-line arguments, allowing for dynamic help messages that reflect the actual executable name being used.

Before:

% firefly

To get started run: ff init
Optional: Set FIREFLY_HOME env variable for FireFly stack configuration path.

Usage:
  ff [command]

After:

% firefly

To get started run: firefly init
Optional: Set FIREFLY_HOME env variable for FireFly stack configuration path.

Usage:
  firefly [command]

Copy link
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

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

I think you are missing a few such as ff start message after an ff init

cmd/root.go Outdated Show resolved Hide resolved
cmd/root.go Outdated Show resolved Hide resolved
@@ -53,7 +56,7 @@ func GetFireflyASCIIArt() string {

// rootCmd represents the base command when called without any subcommands
var rootCmd = &cobra.Command{
Use: "ff",
Use: ExecutableName,
Copy link
Contributor

Choose a reason for hiding this comment

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

We really need to make this usage message better

Copy link
Contributor

Choose a reason for hiding this comment

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

not part of this PR

@EnriqueL8
Copy link
Contributor

You are getting a build error because of
image

cmd/root.go Show resolved Hide resolved
@dwertent
Copy link
Contributor Author

I think the build failed due to a failure in the coverage report generation, maybe we should re-run the build job (it passed last time and the only change I made was the var name)

@EnriqueL8
Copy link
Contributor

I reran it and it's still failing

@EnriqueL8
Copy link
Contributor

I'm quite puzzled about why we are getting a broken pipe, might some related to etcd-io/etcd#17560 or the permission in the workflow

@EnriqueL8
Copy link
Contributor

This is ready to go :)

Copy link
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

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

Looks good - Thanks!

@EnriqueL8 EnriqueL8 merged commit 14e7721 into hyperledger:main Oct 3, 2024
12 checks passed
@dwertent dwertent deleted the exec-name branch October 14, 2024 19:36
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.

2 participants