-
Notifications
You must be signed in to change notification settings - Fork 44
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
Abstract away additionalfeecontribution
to its own struct
#521
Abstract away additionalfeecontribution
to its own struct
#521
Conversation
Pull Request Test Coverage Report for Build 13078047990Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean. I don't think it should be part of the pub
API. No requests other than changing it to private or otherwise clarifying API visibility.
payjoin/src/send/mod.rs
Outdated
@@ -30,12 +30,19 @@ pub mod v2; | |||
|
|||
type InternalResult<T> = Result<T, InternalProposalError>; | |||
|
|||
#[derive(Debug, Clone, Copy, PartialEq, Eq)] | |||
#[cfg_attr(feature = "v2", derive(serde::Serialize, serde::Deserialize))] | |||
pub struct AdditionalFeeContribution { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any good reason for this to be pub
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. Thanks for catching that. Switched to pub(crate)
c149c3b
to
aa22001
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK aa22001
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK aa22001
this commit was drafted in November :O
This tuple was somewhat confusing, and I often forget what each value represents. Organizing this data into a struct makes it more intuitive and easier to work with.
Cherry-pick'd off #434