-
Notifications
You must be signed in to change notification settings - Fork 77
Expose missing sender session history FFI methods #1140
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
Expose missing sender session history FFI methods #1140
Conversation
| /// event log. | ||
| #[derive(uniffi::Object)] | ||
| pub struct SenderSessionStatus { | ||
| inner: payjoin::send::v2::SessionStatus, |
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 inner idiomatic? or should it just be pub struct SenderSessionStatus(payjoin::send::v2::SessionStatus)
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.
There doesn't seem to be a perfectly consistent approach that we take, SenderSessionEvent has a tuple struct while SenderSessionOutcome has named fields, this seems fine imo
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.
I prefer tuple struct where there's only one inner value, but we can make this consistent in a follow-up.
Pull Request Test Coverage Report for Build 18224907273Details
💛 - Coveralls |
status off ffi session history
benalleng
left a comment
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, the different SenderSession Structs in the ffi already accept a variety of tuple vs named fields, it seems fine for this to be this way unless we want to definitively settle on one or the other.
Foreign languages should have access to the send session status after replayling the session event log.
Make PjParam accessible to foreign languages.
d94c1a0 to
3531394
Compare
You are right. The other exports just wrap around the rust-payjoin type. I think thats better bc the foreign language won't have some class with a inner field. |
spacebear21
left a comment
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, receive needs the same treatment (I'll open a PR).
Foreign languages should have access to the send session status after replayling the session event log.
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.