Skip to content

Implement pruneblock method and test #132

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GideonBature
Copy link
Contributor

Going by the conversations with @tcharding for PR #116 on implementing one method at a time for easier review: This is the first method implementation pruneblockchain which is a specific type that returns a standard type (numeric). Once this is approved, I’ll proceed with the remaining ones.

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

Looks pretty good man. Can you strip out all the unrelated changes then I'll do another review. Keep at it man, you're killing it.

self.call("pruneblockchain", &[target.into()])
}
}
};
Copy link
Member

Choose a reason for hiding this comment

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

This is good but can be put it after impl_client_v17__preciousblock? I've tried to keep all the files using the same order for methods that they appear in the help output i.e., types/src/v17/mod.rs (or the rpc-api-v17.txt file).

@@ -49,6 +49,7 @@ crate::impl_client_v17__gettxoutproof!();
crate::impl_client_v17__gettxoutsetinfo!();
crate::impl_client_v17__preciousblock!();
crate::impl_client_v17__verifytxoutproof!();
crate::impl_client_v17__pruneblockchain!();
Copy link
Member

Choose a reason for hiding this comment

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

Same thing here, please keep the calls in order.

Comment on lines -26 to +27
let json: GetBlockVerboseZero = node.client.get_block_verbose_zero(block_hash).expect("getblock verbose=0");
let json: GetBlockVerboseZero =
node.client.get_block_verbose_zero(block_hash).expect("getblock verbose=0");
Copy link
Member

Choose a reason for hiding this comment

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

These look like unrelated formatting changes. Best not to have these.

use node::vtype::*; // All the version specific types.
use node::vtype::*; // All the version specific types.
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated change. Looks like we have our editors configured differently. This is the default spacing set by my editor. Neither is right or wrong its just that this is an unrelated whitespace change.

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