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

Reexport common dependencies from runtime-common #1582

Closed
lemunozm opened this issue Oct 5, 2023 · 2 comments
Closed

Reexport common dependencies from runtime-common #1582

lemunozm opened this issue Oct 5, 2023 · 2 comments
Labels
dependencies Pull requests that update a dependency file. I11-cleaning No mandatory issue that leave the repo more readable/organized P2-nice-to-have Issue is worth doing. Q1-easy Can be done by primarily duplicating and adapting code.

Comments

@lemunozm
Copy link
Contributor

lemunozm commented Oct 5, 2023

Description

Problem

For each pallet/runtime dependency used, we need to add such dependency to:

  • development-runtime
  • altair-runtime
  • centrifuge-runtime
  • integration-tests
  • Sometimes to runtime-common

And configure runtime-benchmarks and try-runtime features in all crates.

Avoiding/mixing some feature names will lead to an annoying compile error that could take time to figure out what's going on.

Solution

We can simplify all these dependency boilerplate by adding all pallets to runtime-common and allowing runtime-common to reexport them. So, in runtime-common we can have:

pub mod reexport {
    pub mod pallets {
        pub use pallet_timestamp;
        // Other pallets
    }
}

From runtimes, on top of the file:

use runtime_common::reexport::pallets::*;

And the code would remain inmutable for runtimes, but with access to all pallets without modifying Cargo.toml files.

In the future, this would avoid nix hash breaks.

@lemunozm lemunozm added Q1-easy Can be done by primarily duplicating and adapting code. P2-nice-to-have Issue is worth doing. dependencies Pull requests that update a dependency file. I11-cleaning No mandatory issue that leave the repo more readable/organized labels Oct 5, 2023
@lemunozm
Copy link
Contributor Author

There are some issues with the reexporting list:

  • The generated weights in runtimes will need to be refactored to prefix the pallet used there with crate:: which implies not to use the default template from substrate and use a custom one that we need to maintain.
  • We need to add use runtime_common::reexport::pallets::*; in every file

@lemunozm
Copy link
Contributor Author

The new usage of workspace dependencies already simplifies this, so I think we can skip this issue by now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file. I11-cleaning No mandatory issue that leave the repo more readable/organized P2-nice-to-have Issue is worth doing. Q1-easy Can be done by primarily duplicating and adapting code.
Projects
None yet
Development

No branches or pull requests

1 participant