Skip to content
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

feat: add contracts feature flag to arbiter-core #592

Merged
merged 8 commits into from
Oct 10, 2023

Conversation

Sabnock01
Copy link
Contributor

Moves contracts/ into arbiter-core and adds a feature flag to the crate to include the contracts.

Resolves #573

@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

Merging #592 (48e46c0) into main (cfb0b5b) will decrease coverage by 1.74%.
Report is 2 commits behind head on main.
The diff coverage is n/a.

❗ Current head 48e46c0 differs from pull request most recent head 4de6f83. Consider uploading reports for the commit 4de6f83 to get more accurate results

@@            Coverage Diff             @@
##             main     #592      +/-   ##
==========================================
- Coverage   60.49%   58.75%   -1.74%     
==========================================
  Files          26       11      -15     
  Lines        5399     3722    -1677     
==========================================
- Hits         3266     2187    -1079     
+ Misses       2133     1535     -598     

see 26 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@Autoparallel Autoparallel left a comment

Choose a reason for hiding this comment

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

In the arbiter-core lib file, we need to annotate the bindings module like so:

#[cfg(feature = "contracts")]
pub mod bindings; 
pub mod data_collection;
pub mod environment;
pub mod math;
pub mod middleware;
#[cfg(test)]
mod tests;

This may interfere with some tests, so we should be careful of that.

Externally, we should find that only if we have:

arbiter-core = { git = "<your github link>", branch = "<your branch name>", features = "contracts" }

are we able to

use arbiter_core::bindings::*;

Perhaps it's better to rename the module to contracts? What do you think of that change as well?

Copy link
Contributor

@Alexangelj Alexangelj left a comment

Choose a reason for hiding this comment

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

I like these changes - this is how it should be done.

I know that the tests rely on the bindings of these contracts @Autoparallel , I think keeping some static bindings in the test folder is how we go about keeping the tests in without relying on the generated bindings from these contracts

@0xJepsen 0xJepsen requested a review from Autoparallel October 8, 2023 16:57
@Autoparallel
Copy link
Collaborator

Nice work!

@Sabnock01
Copy link
Contributor Author

Feel free to squash

@0xJepsen 0xJepsen merged commit c37775d into anthias-labs:main Oct 10, 2023
@Sabnock01 Sabnock01 deleted the sabnock/contracts-feature branch October 11, 2023 03:10
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.

feat: add feature flag to arbiter-core/ crate for contracts/
4 participants