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

Parity to method class #1602

Merged
merged 2 commits into from
Jan 15, 2021
Merged

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented Mar 13, 2020

What was wrong?

The Parity class needed to move over to use the Method class and ModuleV2 class.

How was it fixed?

Started using Method and ModuleV2

Todo:

  • Add entry to the release notes
  • Think about how to get rid of self.web3 = web3 in Module
  • Check that all parity tests have all modules running
  • Typing
  • Cleanup commits

Cute Animal Picture

image

@kclowes kclowes force-pushed the parity-to-method-class branch from a458ac1 to 9a970f8 Compare March 16, 2020 21:38
@kclowes kclowes force-pushed the parity-to-method-class branch 3 times, most recently from c93c650 to 1fdec7f Compare April 16, 2020 21:24
@kclowes kclowes force-pushed the parity-to-method-class branch from 2e4abac to 78af94b Compare April 20, 2020 18:33
@kclowes kclowes changed the title [WIP] Parity to method class Parity to method class Apr 20, 2020
@kclowes kclowes requested a review from pipermerriam April 20, 2020 20:50
@kclowes kclowes force-pushed the parity-to-method-class branch from fbadf4d to af46526 Compare May 11, 2020 18:15
@kclowes kclowes requested review from pipermerriam and wolovim and removed request for pipermerriam May 11, 2020 21:05
@wolovim wolovim force-pushed the parity-to-method-class branch from af46526 to 211b04d Compare July 21, 2020 20:56
Copy link
Member

@wolovim wolovim left a comment

Choose a reason for hiding this comment

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

This may sit for a while longer, but just caught it up with master. Looks good generally and tests are happy, but I don't grok all, so couple of requests for clarification below.

Separate from the intention of this PR, we might be due for a discussion on Parity vs. OpenEthereum support (and maybe other nodes?). Lots of Parity nodes still in use, so obviously we won't chuck that out anytime soon. This topic also comes to mind though after recent All Core Devs conversations about increasing client diversity... Probably worth its own issue.

tests/integration/parity/common.py Show resolved Hide resolved
web3/module.py Outdated Show resolved Hide resolved
@kclowes kclowes force-pushed the parity-to-method-class branch from cda3b17 to fe8e2b8 Compare August 26, 2020 17:27
@kclowes kclowes force-pushed the parity-to-method-class branch 3 times, most recently from b6356d3 to 9bcdc02 Compare October 16, 2020 17:03
@kclowes kclowes requested a review from wolovim October 16, 2020 19:18
@kclowes kclowes changed the title Parity to method class [WIP] Parity to method class Oct 16, 2020
@kclowes kclowes removed the request for review from wolovim October 16, 2020 19:32
@kclowes kclowes force-pushed the parity-to-method-class branch from 9bcdc02 to c8bdbd4 Compare December 18, 2020 21:37
@kclowes kclowes changed the title [WIP] Parity to method class Parity to method class Dec 18, 2020
@kclowes
Copy link
Collaborator Author

kclowes commented Dec 18, 2020

@marcgarreau I think this is (finally) ready to go!

@kclowes kclowes requested a review from wolovim January 8, 2021 21:29
@kclowes kclowes mentioned this pull request Jan 8, 2021
1 task
@kclowes kclowes force-pushed the parity-to-method-class branch 2 times, most recently from 0ebfe76 to 864e13c Compare January 14, 2021 19:36
@kclowes kclowes force-pushed the parity-to-method-class branch from 864e13c to cf57aa8 Compare January 15, 2021 17:00
@kclowes kclowes merged commit 3a416f3 into ethereum:master Jan 15, 2021
@kclowes kclowes deleted the parity-to-method-class branch January 15, 2021 17:10
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