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

docs(request-response): update docs about peer address management #5902

Merged
merged 10 commits into from
Mar 13, 2025
Merged

docs(request-response): update docs about peer address management #5902

merged 10 commits into from
Mar 13, 2025

Conversation

wowinter13
Copy link
Contributor

Description

Fixes #5372

Notes & open questions

Or we can maybe mention Swarm::add_peer_address

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

dariusc93
dariusc93 previously approved these changes Mar 4, 2025
View reviewed changes
Copy link
Member

@dariusc93 dariusc93 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 PR. While im not against removing it, it might make more sense to remove it upon Behaviour::add_address and Behaviour::remove_address being removed.

@elenaf9
Copy link
Contributor

elenaf9 commented Mar 9, 2025

Or we can maybe mention Swarm::add_peer_address

Yes we should leave the note and just change the refs to the Behavior level methods to the Swarm methods. It is still true that users need to either add addresses explicitly, or combine the behavior with another address discovery behavior.

dariusc93 and others added 4 commits March 10, 2025 00:41
@dariusc93 @wowinter13
chore: Add edition to workspace Cargo.toml and inherit workspace edition
2cc6c81 
Add edition to workspace Cargo.toml and Inherits workspace edition. Additionally, inherits rust-version in `libp2p-upnp` and `libp2p-websocket-websys`.

Pull-Request: libp2p#5896.
@Khwahish29 @wowinter13
fix: rename InsufficientPeers to NoPeersSubscribedToTopic for clarity
16e498f 
The `InsufficientPeers` error was unclear about whether it meant no peers were connected at all or no peers were subscribed to the topic. Rename it to `NoPeersSubscribedToTopic` to make it clear that this error occurs when trying to publish to a topic that has no subscribed peers.
Fixes libp2p#5907

Pull-Request: libp2p#5912.
@wowinter13
chore(docs): fix outdated note
3bc34cd
@wowinter13
Merge branch 'master' into remove-outdated-note
d6dff42
@wowinter13
Copy link
Contributor Author

@elenaf9 fixed

@wowinter13 wowinter13 requested a review from elenaf9 March 12, 2025 11:45
elenaf9
elenaf9 previously approved these changes Mar 13, 2025
View reviewed changes
@elenaf9 elenaf9 changed the title chore(docs): remove outdated note docs(request-response): update docs about peer address management Mar 13, 2025
@mergify mergify bot dismissed stale reviews from dariusc93 and elenaf9 March 13, 2025 01:18

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

@elenaf9
Copy link
Contributor

elenaf9 commented Mar 13, 2025

Thanks @wowinter13!

@mergify mergify bot merged commit 3654685 into libp2p:master Mar 13, 2025
70 of 71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elenaf9 elenaf9 elenaf9 approved these changes

@dariusc93 dariusc93 dariusc93 left review comments

Assignees
No one assigned
Labels
send-it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

request-response: Inconsistent documentation.
4 participants
@wowinter13 @elenaf9 @dariusc93 @Khwahish29