Skip to content

Add comments to FFI interface#1653

Open
xstoicunicornx wants to merge 1 commit into
payjoin:masterfrom
xstoicunicornx:ffi-comments
Open

Add comments to FFI interface#1653
xstoicunicornx wants to merge 1 commit into
payjoin:masterfrom
xstoicunicornx:ffi-comments

Conversation

@xstoicunicornx

Copy link
Copy Markdown
Collaborator

Summary

Adds some nice comments to the FFI receive interface in order to have consistent doc strings across type states.

Claude AI was used to collaboratively create many of these comment strings.

Pull Request Checklist

Please confirm the following before requesting review:

@coveralls

coveralls commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 27715375187

Coverage remained the same at 85.224%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 14747
Covered Lines: 12568
Line Coverage: 85.22%
Coverage Strength: 370.26 hits per line

💛 - Coveralls

@caarloshenriq caarloshenriq 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.

ACK 3454b76

@DanGould

Copy link
Copy Markdown
Member

Haven't had the chance to check that these are identical to the underlying docs yet, did you @caarloshenriq ?

Wondering how comment consistency might be automated going forward so that comments stay fresh with the underlying functions they're bound to. Without automation we will run into this issue again for sure.

@xstoicunicornx

Copy link
Copy Markdown
Collaborator Author

I am not sure automation is completely possible just because the interface is inherently slightly different. Might work well with some sort of AI prompt you run every few months? At this point there really shouldn't be large scale changes to the interface right?

As far as matching the PDK doc strings, I would say they don't match in style but are much more consistent and (imo) clear. I actually was planning to submit a PR revamping the PDK doc strings to match the consistency of these FFI doc strings after the async compatible interface got merged just so see what people thought about it, but would be happy to submit that sooner if there is interest in that.

@xstoicunicornx

Copy link
Copy Markdown
Collaborator Author

So I guess what I am saying is that I intentionally did not verbatim copy the existing doc strings because I think the existing doc strings themselves should be improved.

@caarloshenriq

Copy link
Copy Markdown
Contributor

Haven't had the chance to check that these are identical to the underlying docs yet, did you @caarloshenriq ?

Wondering how comment consistency might be automated going forward so that comments stay fresh with the underlying functions they're bound to. Without automation we will run into this issue again for sure.

Yes, I cross-checked each new comment against the core implementation before leaving my own review notes Verifying accuracy, not verbatim match, so the intentional style improvements make sense.

On automation: I think there's no clean native mechanism in Rust/uniffi for FFI wrappers to inherit doc comments from the underlying types. The #[doc(inline)] attribute only works for re-exports within the same crate, not across the FFI boundary.

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.

4 participants