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

ERC20Transfers sort #16

Closed
wants to merge 4 commits into from
Closed

ERC20Transfers sort #16

wants to merge 4 commits into from

Conversation

wimgithub
Copy link

ERC20Transfers sort

@nanmu42
Copy link
Owner

nanmu42 commented Jul 17, 2019

感谢你的贡献。

请修正CI测试,并且gofmt代码。

@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

Merging #16 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #16      +/-   ##
==========================================
+ Coverage   78.73%   78.81%   +0.07%     
==========================================
  Files          10       10              
  Lines         268      269       +1     
==========================================
+ Hits          211      212       +1     
  Misses         34       34              
  Partials       23       23
Impacted Files Coverage Δ
account.go 91.89% <100%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c11586...ff266cb. Read the comment docs.

@nanmu42 nanmu42 self-requested a review July 18, 2019 02:13
Copy link
Owner

@nanmu42 nanmu42 left a comment

Choose a reason for hiding this comment

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

感谢你的贡献。

在考虑你的PR时,我发现你的变更也许需要一些实现风格上的调整,以保持这个库的API风格一致。这一点请参阅详细的评审意见。

最后,请在项目根目录使用 gofmt -w .格式化代码,这非常重要。

@@ -96,6 +96,7 @@ func (c *Client) ERC20Transfers(contractAddress, address *string, startBlock *in
compose(param, "address", address)
compose(param, "startblock", startBlock)
compose(param, "endblock", endBlock)
compose(param, "sort", sort) // asc - 升序,desc - 降序
Copy link
Owner

@nanmu42 nanmu42 Jul 18, 2019

Choose a reason for hiding this comment

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

请使用和这里相同的方式来引入排序方式,从而保持这个库的API风格一致:

etherscan-api/account.go

Lines 35 to 49 in 28455ff

//
// if desc is true, result will be sorted in blockNum descendant order.
func (c *Client) NormalTxByAddress(address string, startBlock *int, endBlock *int, page int, offset int, desc bool) (txs []NormalTx, err error) {
param := M{
"address": address,
"page": page,
"offset": offset,
}
compose(param, "startblock", startBlock)
compose(param, "endblock", endBlock)
if desc {
param["sort"] = "desc"
} else {
param["sort"] = "asc"
}

修改后的该函数形如:

func (c *Client) ERC20Transfers(contractAddress, address, startBlock *int, endBlock *int, page int, offset int, desc bool) (txs []ERC20Transfer, err error) 

代码中请使用英文注释,注释请放在函数级别,这句话也许有帮助:

if desc is true, result will be sorted in blockNum descendant order. 

@nanmu42
Copy link
Owner

nanmu42 commented Aug 5, 2019

Closed due to long period inactivity.

If you have got any update, feel free to reopen this PR.

@nanmu42 nanmu42 closed this Aug 5, 2019
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.

2 participants