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.

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

feat(kad): migrate to quick-protobuf-codec crate #4501

Merged
merged 6 commits into from
Sep 15, 2023
Merged

feat(kad): migrate to quick-protobuf-codec crate #4501

merged 6 commits into from
Sep 15, 2023

Conversation

0xcrust
Copy link
Contributor

@0xcrust 0xcrust commented Sep 13, 2023

Description

Resolves #4487.

Notes & open questions

Much thanks to @thomaseizinger for the very useful pointers!

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Thank you!

Looks good overall, I've left some minor comments :)

protocols/kad/CHANGELOG.md Outdated Show resolved Hide resolved
protocols/kad/src/protocol.rs Outdated Show resolved Hide resolved
protocols/kad/src/protocol.rs Outdated Show resolved Hide resolved
protocols/kad/src/protocol.rs Outdated Show resolved Hide resolved
protocols/kad/src/protocol.rs Outdated Show resolved Hide resolved
protocols/kad/src/protocol.rs Show resolved Hide resolved
protocols/kad/src/protocol.rs Show resolved Hide resolved
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you :)

protocols/kad/CHANGELOG.md Outdated Show resolved Hide resolved
protocols/kad/CHANGELOG.md Outdated Show resolved Hide resolved
protocols/kad/CHANGELOG.md Outdated Show resolved Hide resolved
protocols/kad/Cargo.toml Show resolved Hide resolved
@thomaseizinger
Copy link
Contributor

Seems you missed to checkin changes to the lockfile :)

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Very clean! Thank you for the work.

thomaseizinger
thomaseizinger previously approved these changes Sep 14, 2023
@mergify mergify bot dismissed thomaseizinger’s stale review September 15, 2023 20:16

Approvals have been dismissed because the PR was updated after the send-it label was applied.

@mergify mergify bot merged commit 508cad1 into libp2p:master Sep 15, 2023
69 checks passed
StemCll pushed a commit to StemCll/rust-libp2p that referenced this pull request Sep 17, 2023
thomaseizinger pushed a commit that referenced this pull request Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kad: migrate to quick-protobuf-codec crate
3 participants