Rust bindings (metkit-sys crate)#209
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #209 +/- ##
========================================
Coverage 55.41% 55.41%
========================================
Files 322 322
Lines 13792 13792
Branches 1102 1102
========================================
Hits 7643 7643
Misses 6149 6149 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
dc5b57d to
dbd62c4
Compare
| This crate provides raw FFI bindings using [cxx](https://cxx.rs/). For a safe, ergonomic API, use the higher-level `metkit` crate (planned). | ||
|
|
||
| ## Features | ||
|
|
There was a problem hiding this comment.
Please check, and ensure we only leave actually supported stuff here.
| }; | ||
|
|
||
| // CodesHandle factory functions | ||
| std::unique_ptr<CodesHandleWrapper> codes_handle_from_message(rust::Slice<const uint8_t> data); |
There was a problem hiding this comment.
Should these not just be CodesHandle::from_message(...) and so forth, as static functions?
|
|
||
| // ==================== ParamID / WindFamily ==================== | ||
|
|
||
| size_t wind_family_count(); |
There was a problem hiding this comment.
Can we avoid exposing the wind functionality, unless we really need it.
|
|
||
| // ==================== MarsRequest ==================== | ||
|
|
||
| rust::String MarsRequestWrapper::verb() const { |
There was a problem hiding this comment.
Can we:
- Split this and the header functionally, in the same way that we did in eckit
- Name files appropriately, after the class wrapped (e.g. MarsRequest.cc) and with the right extension
- Make sure we stylistically include formatting (e.g. ------------ separators, header annotations, ...)
And generally make similar to the eckit PR.
There was a problem hiding this comment.
Split large file.
Use naming style.
No more free functions.
Fixed in ef9d316
| bool is_data(rust::Str keyword) const; | ||
| }; | ||
|
|
||
| std::unique_ptr<MarsLanguageWrapper> language_create(rust::Str verb); |
There was a problem hiding this comment.
Some of these functions that should be static factory methods are still free standing (and some have been merged). Can you please check and make sure you are consistent.
Description
Contributor Declaration
By opening this pull request, I affirm the following: