Feature/glue code generation

tchataigner commented 3 years ago

What It Does 🔎

This PR is a starting point of binding generation for holium transformations developed in Rust. It exposes a procedural macro that will wrap functions with necessary code to run in an Holium runtime.

Documentation Updates 📘

Pull Request

How To Test ✔️

cargo test

Linked Issues 🎫

https://polyphene.myjetbrains.com/youtrack/issue/HTB-27

Related Pull Requests 🔀

Proposition of new issues

tchataigner commented 3 years ago

As of now it works with the development started in the repository holium on the branch feature/ee-interface-get

tchataigner commented 2 years ago

Ready for second review round. Created a new ticket as discussed for custom serde Serializer & Deserializer. @PhilippeMts I'll let you specify implementation method & time estimate it.

[ ] https://polyphene.myjetbrains.com/youtrack/issue/HTB-124

PhilippeMts commented 2 years ago

Thank you Thomas for your humongous work on this feature and all the answers you provided to my first comments.

As I prove to be unable to provide useful feedback in due times on this PR, I will only ask you to focus on the following points and we may then move forward.

Repository organization

Personal report

Currently, I may identify a few issues in the way this project is organized:

We have no interest mixing the rust-integration-test crate with other crates, as it is in fact a test asset more than source code for the SDK.
Mixing workspace information with a crate configuration, like it is done in the root Cargo.toml file, does not seems like a safe choice.

Documentation on workspaces: https://doc.rust-lang.org/book/ch14-03-cargo-workspaces.html.

Recommendation

I would thus recommend the following actions:

Create a ./crates/sdk directory.
Move ./src and ./Cargo.toml to ./crates/sdk.
Remove the [workspace] section from this Cargo.toml.
Move ./tests to ./tests/assets/rust-package.

Have a new root Cargo.tml file:

[workspace]
members = [
  "crates/sdk",
  "crates/macro-support",
  "crates/macro",
  "crates/backend",
]

Update the README accordingly.

Miscellaneous

Please we may allow for minor release updates using the ^ character in the holium-macro-support crate.
./src/internal/data_tree.rs:83

You should handle the memory access in a more secure way, using the key_node.children.get method.

tchataigner commented 2 years ago

Mistake on my side while merging, reopening w/ develop as base !

tchataigner commented 2 years ago

Ok @PhilippeMts all comments are taken into account ! Thank you !

polyphene / holium-rs-sdk