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

Devp2p EventEmitter refactor #2893

Merged
merged 44 commits into from
Jul 19, 2023
Merged

Devp2p EventEmitter refactor #2893

merged 44 commits into from
Jul 19, 2023

Conversation

scorbajio
Copy link
Contributor

@scorbajio scorbajio commented Jul 16, 2023 •
edited
Loading

This PR addresses part of #2872 by changing the classes in the devp2p package that extend the EventEmitter class to rather keep the emitter in a class field. This PR builds off of #2889 and should be merged after #2889 has been merged.

scorbajio added 26 commits July 13, 2023 13:49
@scorbajio
Change access specifiers for RLPx _privateKey, _id, _debug, _timeout
3437b24
@scorbajio
Renaming accessed id property in client tests
d6e994e
@scorbajio
Change access specifiers for RLPx _maxPeers, _clientId, _remoteClient…
edd692a 
…IdFilter, and _capabilities
@scorbajio
Ignore error message from reassigning readonly property in tests
640f52b
@scorbajio
Change access specifiers for _common, _listenPort, and _dpt
1b9f734
@scorbajio
Change access specifiers for _peersLRU, _peersQueue, _server, _peers,…
be2d41d 
… _refillIntervalId, and _refillIntervalSelectionCounter
@scorbajio
Ignore accessibility errors in examples
abcd71b
@scorbajio
Update names and access specifiers of Peer fields
666e077
@scorbajio
Ignore access errors for _eciesSession
29849e9
@scorbajio
Make common field public for Peer class
991397c
@scorbajio
Update names and access specifiers of Peer fields
b623ef0
@scorbajio
Make id public readonly
7fcc607
@scorbajio
Update names and access specifiers of Mac class fields
584f5ef
@scorbajio
Update names and access specifiers of ECIES class fields
8c73139
@scorbajio
Update names and access specifiers of Peer class fields
12d266d
@scorbajio
Ignore accessibility errors in examples
00c2cac
@scorbajio
Update example
aa10a2e
@scorbajio
Update names and access specifiers of class fields in protocol subpac…
40b15a1 
…kage
@scorbajio
Update names and access specifiers of class fields in ext subpackage
6c84f17
@scorbajio
Update names and access specifiers of class fields in dpt subpackage
e8e7f22
@scorbajio
Update names and access specifiers of class fields in dns subpackage
2250869
@scorbajio
Merge branch 'master' into devp2p-fields-access-and-naming-cleanup
cea1c28
@scorbajio
Fix name of accessed field
34a1ab9
@scorbajio
Update tests
cc708e2
@scorbajio
Merge branch 'devp2p-fields-access-and-naming-cleanup' of github.com:…
9cfb5b8 
…ethereumjs/ethereumjs-monorepo into devp2p-fields-access-and-naming-cleanup
@scorbajio
Don't extend EventEmitter in RLPx class
12534b6
@codecov
Copy link

codecov bot commented Jul 16, 2023 •
edited
Loading

Codecov Report

Merging #2893 (8866867) into master (9fec1f4) will increase coverage by 0.42%.
The diff coverage is 93.33%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 88.66% <ø> (ø)
blockchain 92.58% <ø> (ø)
client 88.21% <93.33%> (+0.13%) ⬆️
common 97.13% <ø> (ø)
ethash ∅ <ø> (∅)
evm 69.95% <ø> (ø)
rlp ?
statemanager 85.16% <ø> (ø)
trie 90.29% <ø> (ø)
tx 95.96% <ø> (ø)
util 86.77% <ø> (ø)
vm ?

Flags with carried forward coverage won't be shown. Click here to find out more.

@scorbajio scorbajio linked an issue Jul 16, 2023 that may be closed by this pull request
Update devp2p API methods #2872
Closed
2 tasks
@holgerd77
Copy link
Member

Ok, thanks a lot for the other PR 👍, RC releases tomorrow (Tuesday), I might or might not be able to still take this one in as well. Definitely no stress though, as you make it. Otherwise we'll just take this into the final breaking releases.

acolytec3
acolytec3 approved these changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

LGTM!

@acolytec3 acolytec3 merged commit 966d7dd into master Jul 19, 2023
@holgerd77 holgerd77 deleted the devp2p-eventemitter-refactor branch July 20, 2023 07:35
@holgerd77
Copy link
Member

To-tal-ly love this!!! 🙂👍

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acolytec3 acolytec3 acolytec3 approved these changes

Assignees
No one assigned
Labels
breaking package: client package: devp2p PR/Issue state: blocked type: refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update devp2p API methods
3 participants
@scorbajio @holgerd77 @acolytec3