Merged
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
c74e3d7 to
7563042
Compare
7563042 to
ec643e2
Compare
ec643e2 to
1e1d2e3
Compare
There was a problem hiding this comment.
Pull request overview
This pull request replaces the generic IdentTag type with type-safe MonitorTag and DeadlineTag types, improving type safety across the Rust-C++ FFI boundary. Additionally, it refactors the FFI layer to use a consistent error handling pattern with out-parameters and FFICode return values.
Changes:
- Introduced type-safe tag system with
MonitorTagandDeadlineTagto distinguish between different tag types at compile time - Refactored FFI functions to return
FFICodestatus codes and use out-parameters for handles, with comprehensive null-pointer validation - Aligned error code enums between Rust (
FFICode) and C++ (Error) for direct casting - Removed manual error conversion helper (
ffi_helpers.h) in favor of direct enum casting - Added extensive test coverage for all FFI functions including null parameter tests
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/health_monitoring_lib/rust/tag.rs | New module introducing type-safe MonitorTag and DeadlineTag types with FFI-compatible representation |
| src/health_monitoring_lib/rust/ffi.rs | Refactored FFI functions to use out-parameters and FFICode return values, added comprehensive null-checking and tests |
| src/health_monitoring_lib/rust/deadline/ffi.rs | Updated deadline FFI functions to match new pattern, added extensive test coverage |
| src/health_monitoring_lib/rust/lib.rs | Updated API to use type-safe tags, refactored internal methods for better testability |
| src/health_monitoring_lib/rust/common.rs | Removed IdentTag type (moved to tag module as separate types), updated trait signatures |
| src/health_monitoring_lib/rust/deadline/deadline_monitor.rs | Updated to use MonitorTag and DeadlineTag, improved parameter naming |
| src/health_monitoring_lib/rust/worker.rs | Updated imports and tag usage |
| src/health_monitoring_lib/rust/log.rs | Added score_write macro export for tag formatting |
| src/health_monitoring_lib/cpp/include/score/hm/tag.h | New C++ header defining type-safe tag classes with FFI-compatible layout |
| src/health_monitoring_lib/cpp/include/score/hm/common.h | Updated Error enum to align with Rust, added abort_on_error helper |
| src/health_monitoring_lib/cpp/include/score/hm/health_monitor.h | Updated API signatures to use type-safe tags |
| src/health_monitoring_lib/cpp/include/score/hm/deadline/deadline_monitor.h | Updated API signatures to use DeadlineTag |
| src/health_monitoring_lib/cpp/health_monitor.cpp | Implemented new FFI calling conventions with error checking |
| src/health_monitoring_lib/cpp/deadline_monitor.cpp | Implemented new FFI calling conventions with error checking |
| src/health_monitoring_lib/cpp/common.cpp | Added abort_on_error implementation |
| src/health_monitoring_lib/cpp/ffi_helpers.h | Removed (obsoleted by direct enum casting) |
| src/health_monitoring_lib/cpp/tests/health_monitor_test.cpp | Updated test to use type-safe tags |
| examples/rust_supervised_app/src/main.rs | Updated to use MonitorTag and DeadlineTag |
| examples/cpp_supervised_app/main.cpp | Updated to use MonitorTag and DeadlineTag |
| src/health_monitoring_lib/BUILD | Updated build file to include tag.h and remove ffi_helpers.h |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1e1d2e3 to
4297f99
Compare
pawelrutkaq
requested changes
Feb 23, 2026
Replace `IdentTag` with type-safe types.
4297f99 to
a2fec84
Compare
pawelrutkaq
approved these changes
Feb 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace
IdentTagwith type-safe types.