feat: expose PsbtInputsError index in FFI #1305
Conversation
Pull Request Test Coverage Report for Build 21513219768Details
💛 - Coveralls |
|
Thanks for the MR. you can use language like the following next time to link to the relevant issue: closes #1276 |
Signed-off-by: Harsh Dev Pathak <hiddenHaze@proton.me>
Harshdev098
force-pushed
the
expose-PsbtInputError
branch
from
7ed9fd8 to
8a35dc0
Compare
|
Why did you write "Signed-off-by: Harsh Dev Pathak hiddenHaze@proton.me" in your commit? I have never seen that style before. |
|
@DanGould I used git commit -s, which automatically adds the Signed-off-by line as part of the DCO sign-off |
spacebear21
left a comment
spacebear21
left a comment
There was a problem hiding this comment.
https://git-scm.com/docs/git-commit#Documentation/git-commit.txt--s
This isn't necessary for our project. It would be to include some description in the commit message however, following the rules in https://cbea.ms/git-commit/
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { |
There was a problem hiding this comment.
As a general comment / open question, I'm not sure whether unit tests in payjoin-ffi are really useful because it's just wrapper code for the bindings generators to use. IMO it's more useful to test behavior in downstream languages since that's the code that will actually be used. However, writing these tests in every supported downstream language would get really repetitive and tedious. @chavic what do you think?
|
|
||
| #[derive(Debug, PartialEq, Eq)] | ||
| pub(crate) enum InternalPsbtInputError { | ||
| pub enum InternalPsbtInputError { |
There was a problem hiding this comment.
What is the rationale for making these types pub? This one especially is named Internal so doesn't seem like one that should be exposed.
4 participants
Exposed the PsbtInputError to the ffi layer and have added the tests for the implementation
closes #1276
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.