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

Remove grin lib re-exports from util crate #661

Merged
merged 17 commits into from
Aug 8, 2022

Conversation

yeastplume
Copy link
Member

For convenience, the previous version of the code imported all grin libraries into the util crate and re-exported them for the rest of the wallet crates. This was done so that changes to versions or branches could be made in one place.

This is fine if the wallet is the only thing being compiled, but as discovered when working on the gui, importing libs for both grin and grin-wallet causes namespace conflicts.

The only solution is to ensure all grin crates are referenced by their original path, which means that the re-export needs to be removed and the grin libs imported into the wallet crate-by-crate. This unfortunately also means that we now need to update versioning info for builds or local development in each and every crate except for just the util crate.

It would be very useful to figure out a way to make changes for development/build/releases in a single place using whatever tools cargo provides, but that will have to be a future endeavour.

Copy link
Member

@phyro phyro left a comment

Choose a reason for hiding this comment

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

The nature of this one is such that it would result in compile errors if something was off. The changes look good 👍

# grin_api = { git = "https://github.com/mimblewimble/grin", tag = "v5.0.0-beta.2" }

# For bleeding edge
grin_core = { git = "https://github.com/mimblewimble/grin", branch = "master" }
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: there are spaces/indentation that could be removed in this and other toml files

@yeastplume
Copy link
Member Author

Yes, agreed. Thanks for taking a look!

@yeastplume yeastplume merged commit 8384a3e into mimblewimble:master Aug 8, 2022
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.

2 participants