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.

Sign up for GitHub

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

Jump to bottom

Deprecate processLog and getLogs #2872

Merged
merged 3 commits into from Mar 10, 2023
Merged

Deprecate processLog and getLogs #2872

merged 3 commits into from Mar 10, 2023

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented Mar 9, 2023 •
edited

What was wrong?

Need to deprecate processLog and getLogs.

Related to Issue #2862

How was it fixed?

Added warnings and aliases.

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@kclowes kclowes changed the title Deprecate processLog Deprecate processLog and getLogs Mar 9, 2023
@kclowes kclowes force-pushed the depwarning-getlog-processlog branch from 7c71b98 to c8e6f22 Compare March 9, 2023 22:56
@kclowes kclowes marked this pull request as ready for review March 9, 2023 23:47
@kclowes kclowes requested review from fselmo and pacrob March 9, 2023 23:47
pacrob
pacrob approved these changes Mar 10, 2023
View reviewed changes
Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

needs newsfragment, otherwise lgtm!

@kclowes kclowes force-pushed the depwarning-getlog-processlog branch from a076a62 to 4f14e56 Compare March 10, 2023 18:19
fselmo
fselmo approved these changes Mar 10, 2023
View reviewed changes
Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

This lgtm, there's one more internal use of getLogs that is in the test_updating_greeting_emits_event() test that should probably use the snake_case but it also still works and we know it's aliased to that method so I could go either way.

@kclowes kclowes force-pushed the depwarning-getlog-processlog branch from 4f14e56 to d6a978d Compare March 10, 2023 18:36
@kclowes
Copy link
Collaborator Author

kclowes commented Mar 10, 2023

Got 'em! Thanks for the reviews!

@kclowes kclowes merged commit d6fb7a4 into ethereum:v5 Mar 10, 2023
1 check passed
@kclowes kclowes deleted the depwarning-getlog-processlog branch March 13, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fselmo fselmo fselmo approved these changes

@pacrob pacrob pacrob approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kclowes @fselmo @pacrob