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

fix: rename InsufficientPeers to NoPeersSubscribedToTopic for clarity #5912

Merged
merged 8 commits into from
Mar 9, 2025

Conversation

Khwahish29
Copy link
Contributor

@Khwahish29 Khwahish29 commented Mar 5, 2025

Description

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 #5907

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (error variant documentation updated)
  • I have added tests that prove my fix is effective or that my feature works (existing tests cover this change)
  • A changelog entry has been made in the appropriate crates:
    • Added entry to protocols/gossipsub/CHANGELOG.md
    • Bumped version in protocols/gossipsub/Cargo.toml to 0.48.2

Sorry, something went wrong.

@Khwahish29 Khwahish29 changed the title gossipsub: rename InsufficientPeers to NoPeerSubscribedToTopic fix: rename InsufficientPeers to NoPeersSubscribedToTopic for clarity Mar 5, 2025
@Khwahish29 Khwahish29 marked this pull request as ready for review March 5, 2025 20:00
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! Left a comment regarding the version bump.

@dariusc93 dariusc93 requested a review from jxs March 5, 2025 20:22
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.

Can you update the libp2p-gossipsub version in the Cargo.toml workspace? :)

@Khwahish29
Copy link
Contributor Author

Can you update the libp2p-gossipsub version in the Cargo.toml workspace? :)

Done! Please have a look 🚀

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

Hi, and thanks for this. Left a comment

jxs
jxs previously approved these changes Mar 8, 2025
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@jxs jxs added the send-it label Mar 8, 2025
dariusc93
dariusc93 previously approved these changes Mar 8, 2025
@mergify mergify bot dismissed stale reviews from jxs and dariusc93 March 8, 2025 20:28

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

@dariusc93 dariusc93 added send-it and removed send-it labels Mar 9, 2025
@mergify mergify bot merged commit 2d7ce54 into libp2p:master Mar 9, 2025
70 of 71 checks passed
wowinter13 pushed a commit to wowinter13/rust-libp2p that referenced this pull request Mar 9, 2025
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.
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.

Make InsufficientPeers error a bit more self-explanatory?
5 participants