Skip to content
This repository was archived by the owner on Apr 25, 2026. It is now read-only.

Remove unused dependencies#285

Merged
1xstj merged 1 commit into
mainfrom
unused-deps
Jan 12, 2023
Merged

Remove unused dependencies#285
1xstj merged 1 commit into
mainfrom
unused-deps

Conversation

@Nutomic
Copy link
Copy Markdown
Contributor

@Nutomic Nutomic commented Jan 10, 2023

Summary of changes
Changes introduced in this pull request:

  • Move a lot of dependencies to dev-dependencies

I didnt measure how much this speeds up compile time as that would take too long, but the difference should be very significant.

Reference issue to close (if applicable)

Closes

@shekohex
Copy link
Copy Markdown
Collaborator

Remove rocksdb dependency feature which seems unused and is extremely slow to compile

RocksDB is needed, as it is our storage backend for the blockchain.

Switch to stable Rust

We will still need the nightly Rust too, as it is needed while compiling the WASM runtime, otherwise it will fail to compile and ask you to install the nightly Rust version with the right wasm target too.

@1xstj
Copy link
Copy Markdown
Contributor

1xstj commented Jan 10, 2023

Agree with @shekohex comment, the nightly should not be avoided, even though it might compile here it will create problems downstream. Imo we should benchmark the build time against a standard substrate node (https://github.com/substrate-developer-hub/substrate-node-template) i dont think we can get any faster than the template node.

@shekohex
Copy link
Copy Markdown
Collaborator

Moving a lot of dependencies to dev-dependencies makes sense, and we should actually add this as a CI check too.

@Nutomic
Copy link
Copy Markdown
Contributor Author

Nutomic commented Jan 10, 2023

Okay im adding Rocksdb back in. Though this could be put behind a feature flag if there are use cases where it isnt needed.

About nightly, do you know why it should be used? Wasm build seems to work fine on stable. My guess is that a nightly feature was previously used, and then either stabilized or removed from the code.

@shekohex
Copy link
Copy Markdown
Collaborator

About nightly, do you know why it should be used? Wasm build seems to work fine on stable. My guess is that a nightly feature was previously used, and then either stabilized or removed from the code.

It uses your currently nightly Rust version, for building the WASM runtime, See: https://docs.rs/substrate-wasm-builder/6.0.0/substrate_wasm_builder/#prerequisites

@Nutomic
Copy link
Copy Markdown
Contributor Author

Nutomic commented Jan 10, 2023

You are right, the build fails if nightly isnt installed. Reverted those changes.

@Nutomic Nutomic changed the title Remove unused dependencies, switch to stable Rust Remove unused dependencies Jan 10, 2023
@1xstj 1xstj self-requested a review January 12, 2023 20:16
Copy link
Copy Markdown
Contributor

@1xstj 1xstj left a comment

Choose a reason for hiding this comment

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

LGTM

@1xstj 1xstj merged commit 7d185e1 into main Jan 12, 2023
@1xstj 1xstj deleted the unused-deps branch January 12, 2023 20:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants