-
Notifications
You must be signed in to change notification settings - Fork 5k
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 removed
property to Log
type
#4877
Add removed
property to Log
type
#4877
Conversation
added this upon jdevcs' [request](web3#4877 (comment))
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.
@samlaf Please resolve conflicts
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.
Please resolve the conflicts. Rest LGTM.
Pull Request Test Coverage Report for Build 2161428870
💛 - Coveralls |
This PR has been automatically marked as stale beacause it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment. |
Merged into 1.x via #5576 |
* Add `removed` property to `Log` type Fixes #4747 (comment) * Update CHANGELOG.md added this upon jdevcs' [request](#4877 (comment)) * Update CHANGELOG.md Co-authored-by: Junaid <[email protected]> Co-authored-by: Samuel Laferriere <[email protected]>
Attempt at Fixing #4747 (comment)
This is just trying to move forward in fixing this issue. I don't have the time to deep dive into web3js and try to understand how to make sure that this PR is non-breaking, so would like for someone else to help me verify.
I just added the property and ran
tsc --noEmit
inpackages/web3-core
which type checked.However I am not sure if it breaks some of the other web3 packages, since when running
tsc --noEmit
in the root folder, I get a gazillion type errors from the tests, such aswhich comes from
Perhaps we could add
// @ts-ignore
above these lines, but I'm not sure if it wouldn't break the// $ExpectError
commands (maybe only the line before the actual test applies?).