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

seperate cli and cmd main package #564

Closed
wants to merge 2 commits into from
Closed

Conversation

iraj720
Copy link

@iraj720 iraj720 commented Sep 8, 2023

hi @serathius
according to this issue that you have raised im trying to embed bbolt to etcdutl cli :
etcd-io/etcd#15760

i have seperated main functionalities of commands in cli package
now it will be easier for etcdutl to easily import this cli package and embed it to its own
and we wont face any versioning issue in there

if it looks good i can start to integrate etcdutl code based on this.
let me know if you have any better suggestion.
Thanks!

@serathius
Copy link
Member

Looks good, cc @ahrtr

Signed-off-by: nima <[email protected]>
@ahrtr
Copy link
Member

ahrtr commented Sep 11, 2023

I am supportive to move commands into a separate package/directory, but I'd suggest to do such structure change after

After above two things are done, then we can move all commands into a separate package/directory in bbolt 2.0. Afterwards, it would just need very minor change to get it embedded into etcdutl (etcd-io/etcd#15760)

@iraj720
Copy link
Author

iraj720 commented Sep 11, 2023

@ahrtr
seems surgery commands and logger are ready to merge
so meanwhile we are merging them i will migrate all commands to cobra style.
then we can release 1.4 and merge this one then etcdutl will use this one.
is it ok?

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

Explicitly mark this PR as "Request changes".

Let's revisit this PR after the two tasks mentioned in #564 (comment) are done.

@ahrtr ahrtr deleted the branch etcd-io:master December 12, 2023 10:26
@ahrtr ahrtr closed this Dec 12, 2023
@ahrtr
Copy link
Member

ahrtr commented Dec 12, 2023

Sorry, the PR is automatically closed after renaming the master to main. Please feel free to submit a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants