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

Use cobra directly instead of CliWrapper #1920

Open
ClayCheung opened this issue Jan 28, 2021 · 1 comment
Open

Use cobra directly instead of CliWrapper #1920

ClayCheung opened this issue Jan 28, 2021 · 1 comment

Comments

@ClayCheung
Copy link

Is your feature request related to a problem? Please describe.

I'm always frustrated when I read the code of fission-cli, because cliWrapper (pkg/fission-cli/cliwrapper) increase cognitive load on me. I need to read the driver/cobra/cobra.go to understand cliWrapper abstracts what cobra's capabilities are.

When I try to customize fission-cli, the encapsulation of cliWrapper made it difficult for me to add custom features, such as custom error handling.

Also in ISSUE #1899, I try to add some validate for fission-cli, and I found that the validate for fission-cli is not decoupled with complete(code). Also k8s custom resource(CR) lacks validate. I wish the code of validate is shareable to CLI, CR and API.

Describe the solution you'd like

Is it still necessary to use cliWrapper
why not remove the cliWrapper, instead of using cobra directly.

We could use cobra like k8s command line client: kubectl (kubectl create command example)

Additional context

I have read some history of PR and Issues.

  • Before fission 1.5.0, fission-cli use urfavecli as cli driver.
  • In fission 1.5.0 PR#1265, it use cliwrapper(pkg/fission-cli/cliwrapper) to wrap the urfavecli, and make clean separation of CLI command and libraries mentioned in ISSUE Reconstruct project structure #1189.
  • In fission 1.7.0 PR#1385, migrate from urfavecli to cobra, and it still use cliwrapper to wrap the cobra.

I think it's NOT necessary to use cliWrapper after migrating from urfavecli to cobra.

@davidalpert
Copy link

does this issue need some hacktoberfest love?

before looking into this I would ask:

a) is the proposal to remove cliwrapper in favor of using cobra directly approved in principle?

b) is the existing pr #1921 a good start which I should pick up and contribute to or shall I start from main and try again?

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

No branches or pull requests

3 participants