cobra-cli icon indicating copy to clipboard operation
cobra-cli copied to clipboard

refactor: mv cmd/* ./

Open umarcor opened this issue 2 years ago • 9 comments

See #8.

umarcor avatar Sep 14 '22 22:09 umarcor

Since the cobra-cli uses cobra, isn't nice that it follows the same layout as other programs using cobra?

marckhouzam avatar Sep 16 '22 01:09 marckhouzam

@marckhouzam do other programs using cobra have the main function of the CLI separated from all the files defining the commands of the same CLI? I believe this PR makes cobra-cli more consistent with how it's used in practice. See, for instance:

  • https://github.com/helm/helm/tree/main/cmd/helm
    • https://github.com/helm/helm/blob/main/cmd/helm/helm.go#L58
  • https://github.com/docker/cli
    • https://github.com/docker/cli/blob/master/cmd/docker/docker.go#L250

umarcor avatar Sep 16 '22 07:09 umarcor

@marckhouzam do other programs using cobra have the main function of the CLI separated from all the files defining the commands of the same CLI? I believe this PR makes cobra-cli more consistent with how it's used in practice.

You are right about the main.go. I was more curious about the cmd subdir? All the Cobra programs I've seen use it:

  • https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/kubectl/pkg/cmd
  • https://github.com/helm/helm/tree/main/cmd
  • https://github.com/docker/cli/tree/master/cmd
  • https://github.com/k3d-io/k3d/tree/main/cmd
  • https://github.com/cli/cli/tree/trunk/pkg/cmd

marckhouzam avatar Sep 16 '22 12:09 marckhouzam

I believe the main reasons for all of those projects to use a cmd are:

  • Repos do contain additional content.
  • None of the users are expected to install the tools through go install. End-users can download pre-built executables for their platforms, or use their favourite package manager.

However, in the case of cobra, it was decided to move the CLI to a separated repo (context: spf13/cobra#1240 and spf13/cobra#1597), rather than managing it as done in helm, docker, k3d, gh... and AFAIAA pre-built executables were never provided (https://github.com/spf13/cobra/pull/976#discussion_r332256353). Hence, having cmd/cobra-cli in this repo would be weird, because users would need to use go install github.com/spf13/cobra-cli/cmd/cobra-cli@latest. This PR preserves go install github.com/spf13/cobra-cli@latest.

Moreover, in the case of helm and docker, I believe that the cmd subdir is unnecessary. Particularly, I think they misunderstood the structure that cobra was using. It was cobra/cmd, not cmd/cobra. That is cmd should have not been in the path used for building the executable. Now, mv cmd/helm ./ and mv cmd/docker ./ should have no impact at all (apart from adjusting the imports/build commands). See spf13/cobra#1430.

umarcor avatar Sep 16 '22 14:09 umarcor

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

github-actions[bot] avatar Dec 04 '22 00:12 github-actions[bot]

Hello 👋

Maybe we can consider the layout described here https://github.com/golang-standards/project-layout

  • cmd/
    • cobracli/
      • cobracli.go (package main)
  • internal/
    • add.go
    • ......

andreaangiolillo avatar Dec 22 '22 12:12 andreaangiolillo

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

github-actions[bot] avatar Feb 21 '23 00:02 github-actions[bot]

@marckhouzam @jpmcb maybe we should remove the stale bot in this repo as well?

Refs: https://github.com/spf13/cobra/issues/1858, https://github.com/spf13/cobra/pull/1908, https://github.com/spf13/cobra/issues/1910

umarcor avatar Feb 21 '23 16:02 umarcor

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

github-actions[bot] avatar Apr 23 '23 00:04 github-actions[bot]