-
Notifications
You must be signed in to change notification settings - Fork 0
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
Align C FFI with overview #922
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #922 +/- ##
==========================================
- Coverage 49.17% 48.73% -0.45%
==========================================
Files 63 63
Lines 8821 8901 +80
==========================================
Hits 4338 4338
- Misses 4483 4563 +80 ☔ View full report in Codecov by Sentry. |
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.
PR Overview
This PR updates the Pulumi C FFI and example code to align with the new API overview. Key changes include:
- Adding a new function, get_array_as_string, to the common library for stack exports.
- Renaming and refactoring C FFI types and functions (e.g. replacing CustomRegisterOutputId with CustomCompositeOutputId and PulumiEngine with PulumiContext).
- Updating test cases and configuration (in C++ and Docker examples, as well as cbindgen.toml) to reflect the new naming and API structure.
Reviewed Changes
File | Description |
---|---|
examples/common/src/lib.rs | Added a new function to serialize an array from the stack export. |
crates/c_ffi/src/lib.rs | Refactored types and functions to align with the new FFI design, including renaming and API changes. |
crates/c_ffi/Cargo.toml | Removed unused workspace dependencies to match the updated design. |
examples/cpp/tests/test.rs | Updated tests to use the new get_array_as_string and additional output assertions. |
crates/c_ffi/cbindgen.toml | Updated exported names to reflect the new type names. |
examples/docker/tests/test.rs | Modified tests to assert against repo_digest instead of remote_image_id. |
examples/docker/src/lib.rs | Renamed export key from remote_image_id to repo_digest. |
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
crates/c_ffi/src/lib.rs:189
- An unresolved FIXME remains in the invoke resource function, which may indicate incomplete output management. Consider addressing this issue to ensure proper handling.
// inner_engine.outputs.push(raw); //FIXME
ctx: Rc::downgrade(inner), | ||
}; | ||
|
||
// inner_engine.outputs.push(raw); //FIXME |
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 is an unresolved FIXME comment regarding output registration. Please either implement the necessary logic or remove the comment if it's no longer applicable.
// inner_engine.outputs.push(raw); //FIXME | |
inner_engine.outputs.push(output.native.clone()); |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
Closes #896