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

Expose types from sc-service #5855

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

RomarQ
Copy link
Contributor

@RomarQ RomarQ commented Sep 27, 2024

Description

At moonbeam we have worked on a lazy-loading feature which is a client mode that forks a live parachain and fetches its state on-demand, we have been able to do this by duplicating some code from sc_service::client. The objective of this PR is to simplify the implementation by making public some types in polkadot-sdk.

  • Modules:
    • sc_service::client I do not see a point to only expose this type when test-helpers feature is enabled
  • Types:

In my opinion the advantages of making these types public is greater than keeping them as internals, since it gives more flexibility to devs when building their parachain clients.

Integration

Not applicable, the PR just makes some types public.

Review Notes

The changes included in this PR give more flexibility for client developers by exposing important types.

@RomarQ RomarQ changed the title Expose client module from sc-service Expose types from sc-service Sep 27, 2024
@bkchr
Copy link
Member

bkchr commented Sep 27, 2024

  • sc_service::client I do not see a point to only expose this type when test-helpers feature is enabled

This feature should not exist at all...

Could you give some more details on how you implemented this? I mean why do you need to expose these types at all? I would say that you only need to write some Backend that fetches the values on demand (for sure it would not be async and whatever, but we don't support this any way right now).

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