Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

refact(cmd/repo): repo's sub cmds uses new cmd lib #5677

Merged
merged 3 commits into from
Nov 5, 2018
Merged

refact(cmd/repo): repo's sub cmds uses new cmd lib #5677

merged 3 commits into from
Nov 5, 2018

Conversation

qiwaa
Copy link
Contributor

@qiwaa qiwaa commented Oct 27, 2018

Refer: #5664

License: MIT
Signed-off-by: chenminjian 727180553@qq.com

@qiwaa qiwaa requested a review from Kubuxu as a code owner October 27, 2018 05:47
@qiwaa
Copy link
Contributor Author

qiwaa commented Oct 28, 2018

I have already done this. It looks like CI goes wrong.

Stebalien
Stebalien requested changes Oct 28, 2018
View reviewed changes
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

(our CI is a bit overloaded)

@qiwaa
Copy link
Contributor Author

qiwaa commented Oct 29, 2018

I use repoGcCmd as a reference, so repoGcCmd is also wrong... #5687

@qiwaa
Copy link
Contributor Author

qiwaa commented Oct 30, 2018

Hey, @Stebalien . I have made the appropriate changes to the wrong code. Can you help me review my code again?

@qiwaa qiwaa changed the title refact(cmd/repo): repo fsck and repo version uses new cmd lib refact(cmd/repo): repo's sub cmds uses new cmd lib Oct 30, 2018
Stebalien
Stebalien approved these changes Oct 31, 2018
View reviewed changes
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM but blocked on go-ipfs release.

@Stebalien Stebalien mentioned this pull request Oct 31, 2018
Closed
73 tasks
kevina
kevina suggested changes Nov 2, 2018
View reviewed changes
Copy link
Contributor

@kevina kevina left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Several minor things, but one potential API change that I want @Stebalien okay on.

Otherwise LGTM.

@qiwaa qiwaa changed the title refact(cmd/repo): repo's sub cmds uses new cmd lib [WIP]refact(cmd/repo): repo's sub cmds uses new cmd lib Nov 3, 2018
qiwaa added 3 commits November 3, 2018 17:04
@qiwaa
refact(cmd/repo): repo fsck and repo version uses new cmd lib
f7b6ee3 
License: MIT
Signed-off-by: chenminjian <727180553@qq.com>
@qiwaa
refact(cmd/repo): repo verify uses new cmd lib
5afa7eb 
License: MIT
Signed-off-by: chenminjian <727180553@qq.com>
@qiwaa
refact(cmd/repo): normalize fsck cmd name
ff27f9f 
License: MIT
Signed-off-by: chenminjian <727180553@qq.com>
@qiwaa qiwaa changed the title [WIP]refact(cmd/repo): repo's sub cmds uses new cmd lib refact(cmd/repo): repo's sub cmds uses new cmd lib Nov 3, 2018
kevina
kevina approved these changes Nov 3, 2018
View reviewed changes
Copy link
Contributor

@kevina kevina left a comment

Choose a reason for hiding this comment

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

LGTM

@Stebalien Stebalien merged commit 254dda2 into ipfs:master Nov 5, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Stebalien Stebalien Stebalien approved these changes

@kevina kevina kevina approved these changes

@Kubuxu Kubuxu Awaiting requested review from Kubuxu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@qiwaa @Stebalien @kevina