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

refactor: Improve list subcommand error reporting #113

Merged
merged 2 commits into from
Mar 24, 2023

Conversation

DavidNix
Copy link
Contributor

A sample of the new output:

Loaded chains from /Users/davidnix/src/strangelove/heighliner/chains.yaml

acrechain:
  cosmos-sdk@v0.45.10
  ibc-go@v3.2.0

agoric:
  error: not enough repo info; missing organization or repo

akash:
  error: GET https://raw.githubusercontent.com/ovrclk/akash/main/go.mod: 404 Not Found

assetmantle:
  error: GET https://raw.githubusercontent.com/assetmantle/node/main/go.mod: 404 Not Found

axelar:
  cosmos-sdk@v0.45.11
  ibc-go@v4.2.0

Comment on lines -173 to -175
func init() {
rootCmd.AddCommand(BuildCmd())
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel init functions are generally an anti-pattern and should be avoided.

Although it can be useful for unique purposes, it is generally not recommended to use package-level init() functions because they can have unintended side effects and make it difficult to reason about the behavior of the code.

Particularly:

  • Order of execution is not obvious.
  • Global state is generally an anti-pattern.

Comment on lines +20 to +30
var rootCmd = &cobra.Command{
Use: "heighliner",
Short: "Generate docker images for Cosmos chains",
Long: `Welcome to Heighliner, provided by Strangelove Ventures.

This tool can generate docker images for all different release versions
of the configured Cosmos blockchains in chains.yaml`,
}

rootCmd.AddCommand(BuildCmd())
rootCmd.AddCommand(ListCmd())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead, it's super easy to initialize everything scoped to main(). That way, the execution flow (in this case registering subcommands) is in one spot. No indirection.

@DavidNix DavidNix marked this pull request as ready for review March 24, 2023 18:16
@DavidNix DavidNix merged commit adb9ad2 into main Mar 24, 2023
@DavidNix DavidNix deleted the nix/refactor/improve-list-error-handling branch March 24, 2023 18:32
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.

None yet

2 participants