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

separate crates #92

Merged
merged 5 commits into from
Nov 19, 2024
Merged

Conversation

king-11
Copy link
Contributor

@king-11 king-11 commented Nov 14, 2024

fixed #91

@king-11 king-11 changed the title King 11/separate crates separate crates Nov 14, 2024
@king-11 king-11 marked this pull request as draft November 14, 2024 18:13
binaries will be moved into separate crate for utilization
Signed-off-by: Lakshya Singh <[email protected]>
Signed-off-by: Lakshya Singh <[email protected]>
Signed-off-by: Lakshya Singh <[email protected]>
Signed-off-by: Lakshya Singh <[email protected]>
@king-11 king-11 marked this pull request as ready for review November 16, 2024 03:57
@king-11
Copy link
Contributor Author

king-11 commented Nov 16, 2024

ready for review @RCasatta @dpc

Copy link
Owner

@RCasatta RCasatta left a comment

Choose a reason for hiding this comment

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

ACK 5fdab28

There are some warning to be fixed on nix, can be fixed in next PR, this major refactor looks great, thanks!

@@ -157,15 +157,14 @@ mod inner_test {
use crate::bitcoin::Network;
use crate::{iterate, Config};
use std::sync::mpsc::sync_channel;
use test_log::test;
Copy link
Owner

Choose a reason for hiding this comment

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

didn't know about this, it's great!
Ideally would have been better to introduce this in a separate commit

pub skip_prevout: bool,

/// Maximum length of a reorg allowed, during reordering send block to the next step only
/// if it has `max_reorg` following blocks. Higher is more conservative, while lower faster.
/// When parsing testnet blocks, it may be necessary to increase this a lot
#[arg(short, long, default_value = "6")]
#[cfg_attr(feature = "clap", arg(short, long, default_value = "6"))]
Copy link
Owner

Choose a reason for hiding this comment

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

This is great to do now that the crate is separate, but ideally this should have been a move only commit, followed by changes like this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah makes sense I could have kept them separate thanks for insights will try to take care of it in future contributions....one commit one purpose.

@RCasatta RCasatta merged commit 5fdab28 into RCasatta:master Nov 19, 2024
10 checks passed
@king-11 king-11 deleted the king-11/separate-crates branch November 19, 2024 13:28
@king-11 king-11 restored the king-11/separate-crates branch November 21, 2024 12:20
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.

separate crates for lib and bin
2 participants