[parachain-template] runtime API Implementations into mod apis#3817
[parachain-template] runtime API Implementations into mod apis#3817bkchr merged 7 commits intoparitytech:masterfrom
mod apis#3817Conversation
asiniscalchi
changed the title
asiniscalchi
changed the title
asiniscalchi
changed the title
asiniscalchi
changed the title
mod apis
asiniscalchi
changed the title
mod apismod apis
ggwpez
left a comment
ggwpez
left a comment
There was a problem hiding this comment.
I dont see an issue with the current code.
Just refactoring for the sake of it is not useful. There are enough actual issues in this repo that need fixing. Some are listed here https://mentor.tasty.limo/
|
|
||
| Ok(quote!( | ||
| const RUNTIME_API_VERSIONS: #c::ApisVec = #c::create_apis_vec!([ #( #result ),* ]); | ||
| pub const RUNTIME_API_VERSIONS: #c::ApisVec = #c::create_apis_vec!([ #( #result ),* ]); |
There was a problem hiding this comment.
This refactoring also reveals a potential issue with the visibility of the RUNTIME_API_VERSIONS constant. It necessitates placing the VERSION: RuntimeVersion definition on the same level as the impl_runtime_apis macro call.
There was a problem hiding this comment.
Okay I guess making that thing pub is valid. But that is a one-line change...
There was a problem hiding this comment.
This line change looks useful. I'm also wondering how to separate api! from lib.rs. because I don't like a single file too big (we have more than 2k lines...)
There was a problem hiding this comment.
Indeed, while the change to make the RUNTIME_API_VERSIONS constant public might seem minor, it aligns with a broader goal.
The presence of a "god file" exceeding 700 lines in Substrate's parachain template node, leading to runtimes spanning over 2000 lines throughout the ecosystem, highlights a structural inefficiency.
My proposed refactoring, although not functionally essential, is designed with the parachain template node's users in mind, especially considering the template's role as both an example and a starting point for new parachains.
There was a problem hiding this comment.
The proposed refactor beneficially exposes SDK challenges that obstruct the creation of organized runtimes.
|
@bkchr This PR has a minor change for the Runtime macro. Could you help to review this? |
Head branch was pushed to by a user without write access
|
Hi @bkchr, I think label T* is missing |
bkchr
added
the
R0-no-crate-publish-required
…itytech#3817) This PR significantly refactors the runtime API implementations to improve project structure, maintainability, and readability. Key changes include: 1. **Enhancing Visibility**: Adjusts the visibility of `RUNTIME_API_VERSIONS` in `impl_runtime_apis.rs` to `pub`, making it accessible throughout the runtime module. 2. **Centralizing API Implementations**: Introduces a new file, `apis.rs`, within the parachain template's runtime directory. 3. **Streamlining `lib.rs`**: Updates the main runtime library file to reflect these structural changes. It removes redundant API implementations and points `VERSION` to the newly exposed `RUNTIME_API_VERSIONS` from `apis.rs`, simplifying the overall runtime configuration. ### Motivations Behind the Refactoring: - **Improved Project Structure**: Centralizing API implementations in `apis.rs` offers a clearer, more navigable project structure. - **Better Readability**: Streamlining `lib.rs` and reducing clutter enhance readability, making it easier for new contributors to understand the project layout and logic. ### Summary of Changes: - Made `RUNTIME_API_VERSIONS` public in `impl_runtime_apis.rs`. - Added `apis.rs` to centralize runtime API implementations. - Streamlined `lib.rs` to adjust to the refactored project structure.
4 participants
This PR significantly refactors the runtime API implementations to improve project structure, maintainability, and readability. Key changes include:
RUNTIME_API_VERSIONSinimpl_runtime_apis.rstopub, making it accessible throughout the runtime module.apis.rs, within the parachain template's runtime directory.lib.rs: Updates the main runtime library file to reflect these structural changes. It removes redundant API implementations and pointsVERSIONto the newly exposedRUNTIME_API_VERSIONSfromapis.rs, simplifying the overall runtime configuration.Motivations Behind the Refactoring:
apis.rsoffers a clearer, more navigable project structure.lib.rsand reducing clutter enhance readability, making it easier for new contributors to understand the project layout and logic.Summary of Changes:
RUNTIME_API_VERSIONSpublic inimpl_runtime_apis.rs.apis.rsto centralize runtime API implementations.lib.rsto adjust to the refactored project structure.