-
Notifications
You must be signed in to change notification settings - Fork 409
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
Remove Web3 in favor of ethersjs #1083
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## staging #1083 +/- ##
===========================================
- Coverage 78.88% 74.03% -4.85%
===========================================
Files 56 66 +10
Lines 1539 2392 +853
Branches 271 441 +170
===========================================
+ Hits 1214 1771 +557
- Misses 189 391 +202
- Partials 136 230 +94
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
706e6d9
to
8146aa0
Compare
The test was failing because of an ethers.js error: ethers-io/ethers.js#4182 Remove the unnecessary contract deployment and explain the test in detail. The test code and it's title was not really understandable becuase it was mostly copy-pasted from the above test.
Removes the webjs dependency in favor of ethers.js in monitor tests, as well as in monitor. Change ChainMonitor state variables chainId etc. into a single SourcifyChain. Monitor optionally takes a SourcifyChain[] as a parameter to initialize, otherwise gets the monitored chains from sourcify-chains.ts Using SourcifyChain in Monitor causes chainId to be a number. Change the variable type in places and in SourcifyEventManager accordingly.
8146aa0
to
d1f658a
Compare
Is codecov failing because of insufficient coverage? @marcocastignoli |
in lib-sourcify we have a minimum that is:
|
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.
Did you run the monitor test? Just a few comments, my only real concern is about the getAddress
that changes the behavior of the previous function.
The |
oh yes, I totally forgot it 😆 |
c2cba54
to
d4397c7
Compare
Force pushed to clean the code from Please have a short look at the last commit @marcocastignoli |
Draft PR
To keep consistency we should remove all Web3.js dependencies and instead use ethers.js.
View in Huly HI-548