-
Notifications
You must be signed in to change notification settings - Fork 23
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
Workspace dependencies #68
Conversation
✅ Deploy Preview for docs-oz-polkadot canceled.
|
/// The wasm builder is deactivated when compiling | ||
/// this crate for wasm to speed up the compilation. | ||
#[cfg(not(feature = "std"))] | ||
fn main() {} |
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.
can you inform us on why this part is removed, and why it was not necessary?
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.
The build.rs file is intended to compile the runtime in wasm. This code was preventing the correct compilation so I changed it to match every other runtime build.rs I've seen.
I've never seen this in any other substrate project. In the future, I'd suggest PRing the polkadot-sdk build.rs file before making changes like this in the future to get feedback and confirmation that it works as intended.
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.
https://github.com/paritytech/polkadot-sdk/blob/master/cumulus/parachain-template/runtime/build.rs
in case there was a misunderstanding, we didn't make any changes. This is from substrate
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.
Looking good to me.
I have two questions above, but they are not blocking for this PR
Closes #44 by defining all dependencies once in a workspace Cargo.toml.
This improves developer experience for dependency upgrades in 2 ways: