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

Improve feature configuration #494

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

pedromfedricci
Copy link
Contributor

This PR intends to improve the feature selection for some common use cases. Closes: #271.
Users that would want to:

  • 1.1 import http client only;
  • 1.2 import ws client only;
  • 2.1 import http server only;
  • 2.2 import ws server only;
  • 3.1 import http client and server only;
  • 3.2 import ws client and server only;

The behaviour of the current feature flags is preserved, with one exception (clarification bellow), and adds more flags to provide for those above.
The exception: reexporting RpcModule and SubscriptionSink in the path jsonrpsee::types:: was removed as it is also reexported at root level jsonrpsee:: when server + macro are enabled.
This is to remove the path ambiguity, it also reduces the size of the resulting rlib for servers.
I didn't find the reason for having both paths, if there is I will revert it properly.

This table maps the described use cases against current flags and PR flags:
Note: compiled for x86_64-unknown-linux-gnu in debug mode.

use case current flag pr flag rlib size reduction (%)
1.1 client http-client ~ 17
1.2 client ws-client ~ 21
2.1 server http-server ~ 20
2.2 server ws-server ~ 26
3.1 client + server http-client + http-server ~ 11
3.2 client + server ws-client + ws-server ~ 24

Ty.

Copy link
Contributor

@maciejhirsz maciejhirsz left a comment

Choose a reason for hiding this comment

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

Looks good to me!

jsonrpsee/src/lib.rs Outdated Show resolved Hide resolved
jsonrpsee/src/lib.rs Outdated Show resolved Hide resolved
jsonrpsee/src/lib.rs Outdated Show resolved Hide resolved
jsonrpsee/src/lib.rs Outdated Show resolved Hide resolved
@pedromfedricci
Copy link
Contributor Author

hey! I fixed the comments. ty.

@dvdplm
Copy link
Contributor

dvdplm commented Oct 1, 2021

@pedromfedricci ty so much for this! Do you mind sorting out the merge conflicts? :)

@pedromfedricci
Copy link
Contributor Author

oh ok! master changed since I pushed this, didn't notice.
This changes things a bit, would you take a look again? @dvdplm @maciejhirsz
ty!

@maciejhirsz
Copy link
Contributor

Cheers, will merge as soon as CI is done.

@maciejhirsz maciejhirsz merged commit 5762839 into paritytech:master Oct 1, 2021
@dvdplm
Copy link
Contributor

dvdplm commented Oct 1, 2021

/tip small

@substrate-tip-bot
Copy link

Please fix the following problems before calling the tip bot again:

  • Contributor did not properly post their Polkadot or Kusama address. Make sure the pull request has: "{network} address: {address}".

@dvdplm
Copy link
Contributor

dvdplm commented Oct 1, 2021

@pedromfedricci As a small thank you we wanted to give you a tip in DOTs, feel free to comment with your polkadot address as instructed by the bot. :)

@pedromfedricci pedromfedricci deleted the feature-flags branch December 22, 2022 03:13
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.

[jsonrpsee root crate]: more configurations for internal crates through feature flags.
3 participants