Skip to content

p2p: peerstore backed phonebook#5710

Merged
cce merged 16 commits intoalgorand:masterfrom
shiqizng:shiqi/pbpstore
Aug 31, 2023
Merged

p2p: peerstore backed phonebook#5710
cce merged 16 commits intoalgorand:masterfrom
shiqizng:shiqi/pbpstore

Conversation

@shiqizng
Copy link
Copy Markdown
Contributor

peerstore backed phonebook prototype.

Comment thread network/p2p/peerstore/utils.go
@shiqizng shiqizng requested review from a team, Eric-Warehime, algochoi, cce, iansuvak, tzaffi and winder and removed request for a team August 24, 2023 20:21
@shiqizng shiqizng self-assigned this Aug 24, 2023
@shiqizng shiqizng added Team Lamprey p2p Work related to the p2p project Enhancement labels Aug 24, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 24, 2023

Codecov Report

Merging #5710 (f77fa12) into master (0f2ab92) will increase coverage by 0.04%.
Report is 29 commits behind head on master.
The diff coverage is 73.29%.

@@            Coverage Diff             @@
##           master    #5710      +/-   ##
==========================================
+ Coverage   55.17%   55.22%   +0.04%     
==========================================
  Files         465      465              
  Lines       65032    65207     +175     
==========================================
+ Hits        35883    36012     +129     
- Misses      26764    26793      +29     
- Partials     2385     2402      +17     
Files Changed Coverage Δ
network/phonebook.go 91.33% <ø> (ø)
network/p2p/peerstore/utils.go 77.77% <55.55%> (-11.12%) ⬇️
network/p2p/peerstore/peerstore.go 74.28% <74.25%> (-3.50%) ⬇️

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment thread network/p2p/peerstore/peerstore_test.go Outdated
Copy link
Copy Markdown
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

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

Some questions/suggestions

Comment thread network/p2p/peerstore/peerstore.go Outdated
Comment thread network/p2p/peerstore/peerstore.go Outdated
Comment thread network/p2p/peerstore/peerstore.go Outdated
Comment thread network/p2p/peerstore/peerstore.go Outdated
Comment thread network/p2p/peerstore/utils.go
Comment thread network/p2p/peerstore/peerstore.go
Comment thread network/p2p/peerstore/utils.go
Comment thread network/p2p/peerstore/peerstore.go Outdated
shiqizng and others added 4 commits August 29, 2023 11:59
Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>
@shiqizng shiqizng marked this pull request as ready for review August 30, 2023 16:11
Comment thread network/p2p/peerstore/peerstore.go Outdated
Comment thread network/p2p/peerstore/peerstore_test.go Outdated
tzaffi
tzaffi previously approved these changes Aug 31, 2023
Copy link
Copy Markdown
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

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

LGTM

iansuvak
iansuvak previously approved these changes Aug 31, 2023
Copy link
Copy Markdown
Contributor

@iansuvak iansuvak left a comment

Choose a reason for hiding this comment

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

LGTM as well!

Comment thread network/p2p/peerstore/peerstore_test.go Outdated
Comment thread network/p2p/peerstore/peerstore_test.go Outdated
Comment thread network/p2p/peerstore/peerstore.go Outdated
Copy link
Copy Markdown
Contributor

@cce cce left a comment

Choose a reason for hiding this comment

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

Could you remove the duplicate Phonebook interface definition? Otherwise LGTM

@shiqizng shiqizng dismissed stale reviews from iansuvak and tzaffi via 47bd821 August 31, 2023 19:15
shiqizng and others added 3 commits August 31, 2023 15:15
Co-authored-by: cce <51567+cce@users.noreply.github.com>
Co-authored-by: cce <51567+cce@users.noreply.github.com>
cce
cce previously approved these changes Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement p2p Work related to the p2p project Team Lamprey

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants