Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

eth_syncing, fixed #397 #398

Merged
merged 4 commits into from Feb 10, 2016
Merged

eth_syncing, fixed #397 #398

merged 4 commits into from Feb 10, 2016

Conversation

debris
Copy link
Collaborator

@debris debris commented Feb 10, 2016

No description provided.

@debris debris added the A0-pleasereview 🤓 Pull request needs code review. label Feb 10, 2016
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 10, 2016
SyncState::NotSynced | SyncState::Idle => SyncStatus::None,
SyncState::Waiting | SyncState::Blocks | SyncState::NewBlocks => SyncStatus::Info(SyncInfo {
starting_block: U256::from(status.start_block_number),
current_block: U256::from(status.last_imported_block_number.unwrap_or(status.start_block_number)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be blockchain's best block

@arkpar arkpar added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Feb 10, 2016
@@ -49,10 +52,20 @@ impl Eth for EthClient {
}
}

// TODO: do no hardcode default sync status
fn syncing(&self, params: Params) -> Result<Value, Error> {
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like pretty easy to test it
there is a dummy client spawn function in tests::helpers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, I will make a separate pr with api tests :)

@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Feb 10, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 10, 2016
arkpar added a commit that referenced this pull request Feb 10, 2016
@arkpar arkpar merged commit 0f665a6 into master Feb 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants