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

Feat/tanlang/add more cmd / 增加更多功能和命令 #2

Merged
merged 20 commits into from
Jan 9, 2023

Conversation

LinZexiao
Copy link
Contributor

Linked Issue

ref filecoin-project/venus#5304

Detail

新增命令

  • 消息的查询
  • replace 消息
  • 发消息地址管理
  • 查看订单
  • 升级订单
  • 给矿工设置 ask
  • 创建矿工
  • 查看矿工证明dealline
  • 查看扇区详情
  • 扇区续期

@LinZexiao LinZexiao force-pushed the feat/tanlang/add-more-cmd branch from d050820 to e29530a Compare December 29, 2022 01:37
@LinZexiao LinZexiao changed the title Feat/tanlang/add more cmd Feat/tanlang/add more cmd / 增加更多功能和命令 Dec 29, 2022
cmd/cli/deal.go Outdated Show resolved Hide resolved
},
}

var minerSetAskCmd = &cli.Command{
Copy link
Collaborator

Choose a reason for hiding this comment

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

考虑分开来设置,flag 夹杂在一起

Copy link

@diwufeiwen diwufeiwen left a comment

Choose a reason for hiding this comment

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

首先,这个开发速度我觉得很强,非常利害!
建议:

  1. 一个pr不要提如此多,可以分模块命令提
  2. 提交pr的是第一责任人,希望这个pr的命令首先自己测试过的

client/client.go Show resolved Hide resolved
client/client.go Outdated Show resolved Hide resolved
cmd/cli/addr.go Show resolved Hide resolved
cmd/cli/addr.go Show resolved Hide resolved
cmd/cli/addr.go Show resolved Hide resolved
cmd/cli/deal.go Show resolved Hide resolved
cmd/cli/miner.go Outdated Show resolved Hide resolved
cmd/cli/sector.go Show resolved Hide resolved
repo/config/config.go Show resolved Hide resolved
route/route.go Outdated Show resolved Hide resolved
route/wrap.go Outdated

var err error

if ctx.Request.ContentLength > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

这样写感觉还是蛮奇怪的,这意思就是客户端有几种传值的办法?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,尽可能兼容 多种传参方式,因为不能直接判断 是不是带了 body,就只能用间接的方法了

return cid, err
}

func (s *Service) AddrOperate(ctx context.Context, params *AddrsOperateReq) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

待商榷的功能,目前只需要给个更新参数就行,其他的没必要做。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

目前已经实现了的话,不确定要不要就先保留,明确不要再删掉?

}
}

ret, err := s.Messager.GetMessageByUid(ctx, params.MsgId)
Copy link
Contributor

Choose a reason for hiding this comment

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

不用等待?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

不需要等待,这个接口是无状态可重入的, 只要客户端那边 定时轮询就好

route/route.go Outdated

dealGroup := apiV0Group.Group("/deal")
dealGroup.GET("storage", Wrap(s.DealStorageList))
dealGroup.GET("retrieval", Wrap(s.DealRetrievalList))
Copy link
Contributor

Choose a reason for hiding this comment

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

检索的接口目前应该没法判断检索订单到底是谁的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,所以目前返回的结果是所有的,这个要去掉吗?

supply feedback after cmd exec
change logger name to cli from cmd
fix verbose flag
fix err check
@LinZexiao LinZexiao changed the base branch from feat/tanlang/add-ability-to-send-msg to master December 30, 2022 05:06
cmd/cli/addr.go Show resolved Hide resolved
cmd/cli/addr.go Outdated Show resolved Hide resolved
cmd/cli/miner.go Show resolved Hide resolved
Copy link

@diwufeiwen diwufeiwen left a comment

Choose a reason for hiding this comment

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

LGTM

LinZexiao and others added 4 commits January 5, 2023 08:55
return err rather than panic when create client
…client-with-api

Feat/tanlang/replace client with api / 用封装好的 api 代替 httpClient
@LinZexiao LinZexiao merged commit 41aedf2 into master Jan 9, 2023
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.

4 participants