Skip to content

Refactor payjoin-cli v1, v2 modules#198

Merged
DanGould merged 3 commits into
payjoin:masterfrom
DanGould:hex-app
Feb 20, 2024
Merged

Refactor payjoin-cli v1, v2 modules#198
DanGould merged 3 commits into
payjoin:masterfrom
DanGould:hex-app

Conversation

@DanGould

@DanGould DanGould commented Feb 19, 2024

Copy link
Copy Markdown
Member

Note that I used the async-trait crate because we target msrv 1.63.0

msrv 1.70 allows async traits out of the box, so when we target that we can remove the dependency

@DanGould
DanGould force-pushed the hex-app branch 4 times, most recently from efd8b9b to 9516d4d Compare February 19, 2024 15:12
@DanGould
DanGould marked this pull request as ready for review February 19, 2024 15:33
@DanGould

Copy link
Copy Markdown
Member Author

@jbesraa Separating main.rs from library code should let us write cli integration tests to test things like retry v1/v2 and ohttp-relay details a lot more easily without dealing with command-line parsing

@jbesraa jbesraa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Concept ACK.

Really like the new design with the trait and file separation! makes the code much more comprehensive and readable!

Comment thread payjoin-cli/src/main.rs
#[cfg(feature = "v2")]
let is_retry = matches.get_one::<bool>("retry").context("Could not read --retry")?;
#[cfg(feature = "v2")]
let is_retry = matches.get_one::<bool>("retry").unwrap_or(&false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are you planning to take this out in a different PR?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I'll follow up with it's removal. This was focused on the module separation only

@DanGould
DanGould merged commit 3f54e36 into payjoin:master Feb 20, 2024
@DanGould
DanGould deleted the hex-app branch February 20, 2024 17:43
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