Skip to content

Rust bindings (metkit-sys crate)#209

Merged
simondsmart merged 20 commits into
developfrom
rust-bindings
Jun 23, 2026
Merged

Rust bindings (metkit-sys crate)#209
simondsmart merged 20 commits into
developfrom
rust-bindings

Conversation

@Choochmeque

Copy link
Copy Markdown
Contributor

Description

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

@codecov-commenter

codecov-commenter commented May 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.41%. Comparing base (20a5066) to head (1308e45).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Choochmeque Choochmeque marked this pull request as ready for review June 11, 2026 00:13
This crate provides raw FFI bindings using [cxx](https://cxx.rs/). For a safe, ergonomic API, use the higher-level `metkit` crate (planned).

## Features

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.

Please check, and ensure we only leave actually supported stuff here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 24f32ef

};

// CodesHandle factory functions
std::unique_ptr<CodesHandleWrapper> codes_handle_from_message(rust::Slice<const uint8_t> data);

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.

Should these not just be CodesHandle::from_message(...) and so forth, as static functions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bf34895


// ==================== ParamID / WindFamily ====================

size_t wind_family_count();

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.

Can we avoid exposing the wind functionality, unless we really need it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 92b6a8f


// ==================== MarsRequest ====================

rust::String MarsRequestWrapper::verb() const {

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.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

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.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ef9d316

@simondsmart simondsmart merged commit e9e99a9 into develop Jun 23, 2026
197 of 204 checks passed
@simondsmart simondsmart deleted the rust-bindings branch June 23, 2026 08:58
@Choochmeque Choochmeque restored the rust-bindings branch June 23, 2026 21:16
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.

3 participants