-
Notifications
You must be signed in to change notification settings - Fork 16
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
Move code to "macros" and "session" features #110
Conversation
what exactly is the dependency cycle? drink doesn't depend on ink!, does it? |
it does through contract-transcode (that is only required by feature session after this PR), we don't need all the session stuff in ink! as well |
Exactly.
|
drink/src/lib.rs
Outdated
#[cfg(feature = "session")] | ||
pub use drink_test_macro::{contract_bundle_provider, test}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
am I correct, that it is necessary to put only contract_bundle_provider
behind feature gate? test
should be fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just looked and the drink_test_macro
crate also brings in contract-build
and contract-metadata
crates which would also cause a circular dependency, so maybe we should make drink_test_macro
a dependency feature of drink
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, yeah, that sounds more reasonable... especially that I don't like hiding macros behind session
feature
so I suggest introducing second default feature macros
and feature-gate reexport here (and the dependency as well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I am testing this with ink! fyi, to make sure we don't forget stuff.
Quick question we should also build with just the std feature (the one we will use in ink!)
What's the best way to check that the code does not generate warning
just a new step in the rust-checks.yml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what warning do you have in mind? and yes, I guess adding some check to CI is the best idea
Moved bundle and mocking under session.
Alternatively we could keep it where it is but put this code behind other feature flags
This change is needed, cause the current setup introduce dependency cycle with ink!.
ink! should import drink! without the session feature to avoid creating a dependency cycle