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

Add Snake Case Methods for JSON-RPC calls #1429

Closed
9 tasks done
kclowes opened this issue Aug 22, 2019 · 22 comments
Closed
9 tasks done

Add Snake Case Methods for JSON-RPC calls #1429

kclowes opened this issue Aug 22, 2019 · 22 comments

Comments

@kclowes
Copy link
Collaborator

kclowes commented Aug 22, 2019

What was wrong?

We'd like to start using pythonic snake_case names for all JSON-RPC methods. The pattern has been laid out in the Geth admin module.

How can it be fixed?

Add deprecation warnings, new snake case methods, and tests for all other modules. Ideally, each module would be its own PR.

  • Eth
  • GethShh
  • GethPersonal
  • GethTxPool
  • GethMiner
  • Net
  • ParityShh
  • ParityPersonal
  • Parity
@Patil2099
Copy link
Contributor

@kclowes Can You Explain it a bit more? I want to work on this issue!

@pipermerriam
Copy link
Member

@Patil2099 see this PR #1326 which implements this in the geth admin module. You would follow the same pattern for the listed modules in this issue.

@Patil2099
Copy link
Contributor

Patil2099 commented Aug 22, 2019 via email

@Patil2099
Copy link
Contributor

@kclowes I will start working on it.

@Patil2099
Copy link
Contributor

@kclowes Do I have to do it like this?

@kclowes
Copy link
Collaborator Author

kclowes commented Aug 23, 2019

@kclowes Do I have to do it like this?

I'm not sure what you mean. Can you explain a little more?

We would prefer that you follow the implementation laid out in #1326, but we're open to hearing other ideas!

@Patil2099
Copy link
Contributor

@kclowes should I start working on other modules?
Now I have a good example to follow.

@kclowes
Copy link
Collaborator Author

kclowes commented Sep 26, 2019

That would be great, thank you!

@Patil2099 Patil2099 mentioned this issue Oct 1, 2019
@smilegodly
Copy link

smilegodly commented Oct 5, 2019

@kclowes I'd like to work on the GethPersonal module if that's ok ?

@kclowes
Copy link
Collaborator Author

kclowes commented Oct 5, 2019

@smilegodly that would be great, thanks! Let me know if you have any questions!

@smilegodly
Copy link

smilegodly commented Oct 8, 2019

@kclowes
How can I individually run these tests here, so that I can see if the Deprecation Warnings get raised?


At the moment , I've just been using tox to test on the whole build structure.. but I would like to be able to run just what I need to be running to see that the deprecation warning tests are working fine .

I have figured out that I needed the pytest module and I am now able to run tests! :)

For some reason when testing the admin module the tests for that module (test node_info , test_addPeer...) are coming out as "SKIPPED".
Is there any reason for this happening.. How should I be properly testing these ?

@kclowes
Copy link
Collaborator Author

kclowes commented Oct 9, 2019

@smilegodly If you’re running the core tests, those use eth-tester and the skipped methods probably aren’t implemented in eth-tester. You might try running the integration tests. I think some of those are marked xfail, but the test should run and you may be able to check for a deprecation warning in the test anyway with a ‘with pytest.warns(DeprecationWarning, match=...)’. If that’s confusing I can expand more, just let me know!

@ashish10677
Copy link
Contributor

Is this issue still open?

@kclowes
Copy link
Collaborator Author

kclowes commented Feb 10, 2020

@ashish10677 Yes it is! If you're going to work on one of the modules, please share which one here so we don't get two people working on the same thing! Thanks!

@billbsing
Copy link
Contributor

If it's OK I would like to start on GethMiner.

@billbsing
Copy link
Contributor

I'm working on the Net module

@tmckenzie51
Copy link
Contributor

I'm working on the parity personal module

@billbsing
Copy link
Contributor

If it's ok I would like to start working on the eth module start soon.

I assume @Patil2099 your last PR #1465 is not active anymore?

I'm planning to break it into several PRs since it's the bigger module.

  • properties
  • methods
  • Move to ModuleV2 and the _utils/eth.py module ? (maybe a new issue for this?)

@kclowes
Copy link
Collaborator Author

kclowes commented Mar 5, 2020

Thanks @billbsing, but I've been working on moving the eth module to use ModuleV2, so hold off on that for a bit, otherwise I'll have some nasty merge conflicts! I'll let you know when I'm done if you still want to pick it up!

This was referenced Apr 23, 2021
@wolovim
Copy link
Member

wolovim commented Apr 28, 2021

pretty sure #1963 was the last of em 🎉 closing out.

@wolovim wolovim closed this as completed Apr 28, 2021
@wolovim wolovim moved this from In progress to Done in kanban [experimental] Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

9 participants