Skip to content

refactor generated command & subcommand boilerplate#14

Open
jxsl13 wants to merge 1 commit into
spf13:mainfrom
jxsl13:boilerplate-constructors
Open

refactor generated command & subcommand boilerplate#14
jxsl13 wants to merge 1 commit into
spf13:mainfrom
jxsl13:boilerplate-constructors

Conversation

@jxsl13

@jxsl13 jxsl13 commented Mar 1, 2022

Copy link
Copy Markdown
  • This is not my final image of how the commands should be constructed but my final image would require to work with the Go AST library which I am not familiar with and imo might be overkill.
  • I usually do call rootCmd.AddCommand(NewSubCommand()) inside of the constructor NewRootCmd() which would be quite hard to do with generated code. cobra-cli would need to actually parse generated Go code and append lines at specific locations for that to work with sub commands that are added afterwards.

That's way too much work so I did go a slightly easier way.
Might not be the final stage but definitely a step forward.

@jxsl13

jxsl13 commented Mar 14, 2022

Copy link
Copy Markdown
Author

@johnSchnake please review

@jpmcb jpmcb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi there - thanks for providing this.

Overall, looks good but I'm wondering if we want to do some work to define the recommended approach in our docs and examples before we go forward with merging this.

I like the idea of removing the init chunks

John S. has good eyes on this too, so would also like his input.

I think @marckhouzam can give some recommendation here too as to what Helm does.

@github-actions

Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@marckhouzam

Copy link
Copy Markdown
Collaborator

Still haven't gotten around to this one. Sorry.

@jpmcb

jpmcb commented May 14, 2022

Copy link
Copy Markdown
Collaborator

No worries!!!! We can mark this for post v1.5.0 if that makes sense?

@github-actions

Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@jxsl13

jxsl13 commented Jul 14, 2022

Copy link
Copy Markdown
Author

blub

@github-actions

Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@github-actions

Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@github-actions

Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@github-actions

Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@github-actions

github-actions Bot commented Jun 1, 2023

Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

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.

3 participants