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

chore: expose hashed_bytes of KBucketKey #4698

Merged
merged 3 commits into from
Oct 26, 2023

Conversation

maqi
Copy link
Contributor

@maqi maqi commented Oct 20, 2023

We found it is useful to have the hashed_bytes of KBucketKey to be exposed,
when tracking down the network addressing and data distribution.

Description

Notes & open questions

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

@maqi maqi force-pushed the expose_hashed_bytes_of_KBucketKey branch 3 times, most recently from b27846b to 56a09ce Compare October 20, 2023 11:06
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.

Thanks for the contribution! One comment :)

protocols/kad/src/kbucket/key.rs Outdated Show resolved Hide resolved
@maqi maqi force-pushed the expose_hashed_bytes_of_KBucketKey branch 2 times, most recently from f84b5fa to 7d0a02a Compare October 24, 2023 08:14
@mergify
Copy link
Contributor

mergify bot commented Oct 25, 2023

This pull request has merge conflicts. Could you please resolve them @maqi? 🙏

mxinden
mxinden previously approved these changes Oct 25, 2023
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.

👍 works for me.

@maqi maqi force-pushed the expose_hashed_bytes_of_KBucketKey branch from 7d0a02a to 7d11d8d Compare October 25, 2023 11:04
@maqi
Copy link
Contributor Author

maqi commented Oct 25, 2023

This pull request has merge conflicts. Could you please resolve them @maqi? 🙏

PR rebased and conflict resolved

thomaseizinger
thomaseizinger previously approved these changes Oct 26, 2023
@mergify
Copy link
Contributor

mergify bot commented Oct 26, 2023

This pull request has merge conflicts. Could you please resolve them @maqi? 🙏

@mergify mergify bot dismissed stale reviews from mxinden and thomaseizinger October 26, 2023 22:58

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

@mergify mergify bot merged commit 357ed40 into libp2p:master Oct 26, 2023
71 checks passed
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.

3 participants