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

chore: Binaries incmd folder, examples in examples folder #501

Merged
merged 11 commits into from
Jun 7, 2022

Conversation

orpheuslummis
Copy link
Contributor

@orpheuslummis orpheuslummis commented Jun 5, 2022

RELEVANT ISSUE(S)

Resolves #500

DESCRIPTION

As described in the issue:

Use /cmd/ as the place for all our binaries, defradb, cmd docs generator, man pages generator (upcoming), etc. CLI remains as a package under /cli/. This is scalable and idiomatic to Go. Additionally introduces convenience for the cmd docs generator.

HOW HAS THIS BEEN TESTED?

Manual testing.

CHECKLIST:

  • I have commented the code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the repo-held documentation.
  • I have made sure that the PR title adheres to the conventional commit style (subset of the ones we use can be found under: tools/configs/chglog/config.yml

ENVIRONMENT / OS THIS WAS TESTED ON?

Please specify which of the following was this tested on (remove or add your own):

  • Arch Linux
  • Debian Linux
  • MacOS
  • Windows

@orpheuslummis orpheuslummis added code quality Related to improving code quality action/no-benchmark Skips the action that runs the benchmark. area/cli Related to the CLI binary labels Jun 5, 2022
@orpheuslummis orpheuslummis added this to the DefraDB v0.3 milestone Jun 5, 2022
@orpheuslummis orpheuslummis self-assigned this Jun 5, 2022
Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

Need to change all the files under cli/ that have the package name cmd to cli

cmd/defradb/main.go Outdated Show resolved Hide resolved
cmd/gencmddocs/gencmddocs.go Outdated Show resolved Hide resolved
@orpheuslummis orpheuslummis requested a review from jsimnz June 6, 2022 16:47
README.md Outdated Show resolved Hide resolved
@orpheuslummis orpheuslummis changed the title chore: Unify binaries into the cmd folder chore: Binaries incmd folder, examples in examples folder Jun 7, 2022
Makefile Outdated Show resolved Hide resolved
Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

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

LGTM! made a minor non-blocking follow up comment.

Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

LGTM.

Thought: Not a blocker on this PR, but was wondering if there is a benefit if we included a subcommand on the main defra binary called tools which may include docsgen, mangen, and future cli-completions stuff.

Can be done in a diff PR down the line tho.

@orpheuslummis
Copy link
Contributor Author

Thought: Not a blocker on this PR, but was wondering if there is a benefit if we included a subcommand on the main defra binary called tools which may include docsgen, mangen, and future cli-completions stuff.

My first impression is that I'm in favor of it. It would allow to provide infrequently used tools in a CLI sub-namespace. As I understand it would require a different pattern than what is introduced in this PR. Taking note of it as a potential idea for CLI structure. Thanks.

@orpheuslummis orpheuslummis merged commit 7907b41 into develop Jun 7, 2022
@orpheuslummis orpheuslummis deleted the orpheus/chore/unify-cmd branch June 7, 2022 20:28
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. area/cli Related to the CLI binary code quality Related to improving code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify binaries into a cmd folder
3 participants