-
Notifications
You must be signed in to change notification settings - Fork 77
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
feat: remove duplication of web3 client verrsion method #3508
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: nikolay <[email protected]>
Test Results 22 files - 1 314 suites - 21 47m 45s ⏱️ - 15m 17s For more details on these failures, see this check. Results for commit c1da700. ± Comparison against base commit 11c93ff. This pull request removes 2 tests.
♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Approved but however, we should also reach out to DAs to make sure this removal is not blocking anyone |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, do we know why we added it in the first place?
Signed-off-by: nikolay <[email protected]>
|
TBH I don't know. I tried to dig deeper but it was added a long time ago (September 2022) and I didn't find any clue. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3508 +/- ##
==========================================
+ Coverage 85.43% 85.53% +0.09%
==========================================
Files 69 69
Lines 4743 4741 -2
Branches 1000 1000
==========================================
+ Hits 4052 4055 +3
+ Misses 401 399 -2
+ Partials 290 287 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Description:
Currently, there are two implementations of the web3 client version method.
Solution:
Research why we've added both of them (is the first one deprecated) and remove the non-standard one.
Related issue(s):
Fixes #3505
Notes for reviewer:
After some research, we found out that there is no
web3_client_version
in any JSON RPC standard even in the deprecated ones.Checklist