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

Refactor/lib restructure #37

Merged
merged 5 commits into from
Jan 18, 2023
Merged

Refactor/lib restructure #37

merged 5 commits into from
Jan 18, 2023

Conversation

boyswan
Copy link
Contributor

@boyswan boyswan commented Jan 17, 2023

No description provided.

@Maar-io
Copy link
Contributor

Maar-io commented Jan 17, 2023

name example/simple is misleading. This is the most complex example with all crates included

@Maar-io
Copy link
Contributor

Maar-io commented Jan 17, 2023

Can you make it so that example imports crates it need to use. Now in example contract the rmrk crate is included which then includes all other.
Perhaps creating one more example with only minting and nesting will show the best way to do that.

@boyswan
Copy link
Contributor Author

boyswan commented Jan 17, 2023

Can you make it so that example imports crates it need to use. Now in example contract the rmrk crate is included which then includes all other. Perhaps creating one more example with only minting and nesting will show the best way to do that.

IMO it's better to keep to a single entry crate (much like ink! and openbrush) rather than users having to depend on crates individually. Maybe we could put crates behind features flags so users only pull in the sub-crates they need?

We could have something along the lines of:

rmrk = { path = "..", default-features = false, features = ["nesting", "minting", "base", "equippable", "multiasset"] }

Maybe it makes more sense to decide what logical groups of crates are actual features (ie. equippable actually depends on multiasset and nesting). Such as:

default = ["common", "minting"] 
equippable  = ["nesting", "equippable", "multiasset"] 
# etc...

@PierreOssun
Copy link
Contributor

@boyswan agree. Let's create features with logical group of crate as you mentioned and keep a single entry crate

@boyswan boyswan marked this pull request as ready for review January 18, 2023 12:07
Copy link
Contributor

@PierreOssun PierreOssun left a comment

Choose a reason for hiding this comment

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

LGTM

@boyswan boyswan merged commit 8fcdb54 into main Jan 18, 2023
@boyswan boyswan deleted the refactor/lib-restructure branch January 18, 2023 13:51
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