Skip to content

feat: add rest of Hydra commands#235

Merged
aeneasr merged 1 commit intomasterfrom
feat/hydra-cli
Oct 10, 2022
Merged

feat: add rest of Hydra commands#235
aeneasr merged 1 commit intomasterfrom
feat/hydra-cli

Conversation

@zepatrik
Copy link
Copy Markdown
Member

@zepatrik zepatrik commented Oct 5, 2022

No description provided.

@zepatrik zepatrik requested a review from aeneasr October 5, 2022 10:30
@zepatrik zepatrik marked this pull request as draft October 5, 2022 10:30
@zepatrik zepatrik requested a review from hperl October 5, 2022 10:31
@zepatrik zepatrik marked this pull request as ready for review October 6, 2022 13:51
Copy link
Copy Markdown
Contributor

@hperl hperl left a comment

Choose a reason for hiding this comment

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

I really like the refactoring! I left two minor questions & comments, but LGTM!

"github.com/ory/x/cmdx"
)

func wrapHydraCmd(newCmd func() *cobra.Command) *cobra.Command {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit, should this go into client.go?


func NewIntrospectCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "introspect",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would the more general verb describe also make sense here? I'd imagine that describe would fit more objects than introspect, if we want to follow the kubectl model. But I don't know the semantics for sure.

@aeneasr aeneasr merged commit 9c9848b into master Oct 10, 2022
@aeneasr aeneasr deleted the feat/hydra-cli branch October 10, 2022 08:03
kevgo pushed a commit that referenced this pull request Oct 26, 2022
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