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

proc-macros: Support deprecated methods for rpc client #570

Merged
merged 3 commits into from
Nov 21, 2021

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented Nov 21, 2021

Calling a deprecated method of the RPC client should warn
the user at compile-time.

Extract the #[deprecated] macro as is while parsing the
RpcMethod, and pass through the macro to the RPC client
rendering.

Testing

To ensure that the test will fail during compilation,
warnings are denied.

Check that the deprecate macro will generate warnings
just for the methods that are utilized.

Calling a deprecated method of the RPC client should warn
the user at compile-time.

Extract the `#[deprecated]` macro as is while parsing the
RpcMethod, and pass through the macro to the RPC client
rendering.
To ensure that the test will fail during compilation,
warnings are denied.

Check that the deprecate macro will generate warnings
just for the methods that are utilized.
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.

Code looks super clean despite proc macros being notoriously hard to navigate, and tests are spot on so this is definitely a pass from me! :)

I'm looking over the issue now and I'm a bit confused about what the intended deprecation message should be there (as in, should have a log on the server, or send something in the JSON response?), but this is definitely a valid interpretation! If there is anything we want to change we can do that outside of this PR.

Cheers, and have a great Sunday :)

Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

great, thanks

//cc @dvdplm tip plz :)

@niklasad1 niklasad1 merged commit 9a3c1e9 into paritytech:master Nov 21, 2021
@dvdplm
Copy link
Contributor

dvdplm commented Nov 22, 2021

@lexnv Good stuff here, tyvm. Can you please edit the description of the PR adding your polkadot address on a separate line like so polkadot address: <SS58 Address> and I'll invoke the tip-bot as a small sign of appreciation! :)

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.

4 participants