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: mv cmd/* ./ #48

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: mv cmd/* ./ #48

wants to merge 1 commit into from

Conversation

umarcor
Copy link
Contributor

@umarcor umarcor commented Sep 14, 2022

See #8.

@marckhouzam
Copy link
Collaborator

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

@umarcor
Copy link
Contributor Author

umarcor commented Sep 16, 2022

@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:

@marckhouzam
Copy link
Collaborator

@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:

@umarcor
Copy link
Contributor Author

umarcor commented Sep 16, 2022

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 (spf13/cobra#976 (comment)). 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.

@github-actions
Copy link

github-actions bot commented Dec 4, 2022

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

@andreaangiolillo
Copy link
Contributor

andreaangiolillo commented Dec 22, 2022

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
    • ......

@github-actions
Copy link

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

@umarcor
Copy link
Contributor Author

umarcor commented Feb 21, 2023

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

Refs: spf13/cobra#1858, spf13/cobra#1908, spf13/cobra#1910

@github-actions
Copy link

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.

None yet

3 participants