Initial UniFFI C++ Bindgen - #1389
alan-george-lk wants to merge 7 commits into
Conversation
54329d0 to
a6646fa
Compare
3674716 to
a2f8242
Compare
a2f8242 to
e7bc2c4
Compare
| # uniffi-exposing crates (livekit-ffi, livekit-uniffi, ...). It has no library | ||
| # target — the bindgens read metadata from an already-compiled cdylib via | ||
| # `--library`, so they never depend on the crates they generate for. | ||
| # Tooling crate: each bin is a code-generation CLI shared across the workspace. |
There was a problem hiding this comment.
I think AI adjusted this because I told it I wasn't leveraging livekit-uniffi anymore, I can revert
There was a problem hiding this comment.
Yea I think reverting that makes sense!
| # uniffi-exposing crates (livekit-ffi, livekit-uniffi, ...). It has no library | ||
| # target — the bindgens read metadata from an already-compiled cdylib via | ||
| # `--library`, so they never depend on the crates they generate for. | ||
| # Tooling crate: each bin is a code-generation CLI shared across the workspace. |
There was a problem hiding this comment.
Yea I think reverting that makes sense!
| const REPOSITORY: &str = "https://github.com/alan-george-lk/uniffi-bindgen-cpp.git"; | ||
| const REVISION: &str = "7d54c1c56e0e4c46772ef4c2b5be3cb26396cbdb"; |
There was a problem hiding this comment.
suggestion: If this will be long lived, it probably makes sense to move this under https://github.com/livekit and move to an explicit tag once it is stable?
There was a problem hiding this comment.
Totally on board with that, should we make that move now before this gets in? Might be good in general to have a fork sync come from LiveKit vs. my personal account anyways
There was a problem hiding this comment.
Yea my vote would be to move it over to the livekit organization before merging this.
For reference I recently did something similar for the dart bindgen but moved back to the upstream (due to getting all the fixes upstreamed) before merging.
There was a problem hiding this comment.
I agree, lets move it under livekit
Changeset ✓This PR includes a changeset covering all affected packages:
|
There was a problem hiding this comment.
Devin Review found 1 potential issue.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
Summary
livekit-ffivia Nord Bindgen fork workTesting