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(gossipsub): make gossipsub modules private #3777

Merged
merged 19 commits into from
Apr 23, 2023

Conversation

tcoratger
Copy link
Contributor

@tcoratger tcoratger commented Apr 12, 2023

Description

Resolves #3494.

Notes & open questions

Change checklist

  • I have performed a self-review of my own code
  • 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.

From memory, I think at least some types of the metrics module appear in the public API of gossipsub::Behaviour.

Can you spot check that and re-export them from the root crate?

@tcoratger
Copy link
Contributor Author

From memory, I think at least some types of the metrics module appear in the public API of gossipsub::Behaviour.

Can you spot check that and re-export them from the root crate?

Done

@thomaseizinger
Copy link
Contributor

I pushed some follow-up commits. Should be good to merge now!

@mergify
Copy link
Contributor

mergify bot commented Apr 14, 2023

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

dependabot bot and others added 3 commits April 14, 2023 19:09
This PR renames some method names that don't follow Rust naming conventions or behave differently from what the name suggests:
- Enforce "try" prefix on all methods that return `Result`.
- Enforce "encode" method name for methods that return encoded bytes.
- Enforce "to_bytes" method name for methods that return raw bytes.
- Enforce "decode" method name for methods that convert encoded key.
- Enforce "from_bytes" method name for methods that convert raw bytes.

Pull-Request: libp2p#3775.
Previously, we closed the entire connection upon receiving too many upgrade errors. This is unnecessarily aggressive. For example, an upgrade error may be caused by the remote dropping a stream during the initial handshake which is completely isolated from other protocols running on the same connection.

Instead of closing the connection, set `KeepAlive::No`.

Related: libp2p#3591.
Resolves: libp2p#3690.

Pull-Request: libp2p#3625.
@tcoratger
Copy link
Contributor Author

I pushed some follow-up commits. Should be good to merge now!

Yes I have just merged from master as there was conflicts but then should be good for merging, what do you think?

thomaseizinger
thomaseizinger previously approved these changes Apr 15, 2023
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!

@mergify mergify bot dismissed thomaseizinger’s stale review April 23, 2023 20:45

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

@mergify mergify bot merged commit 270d204 into libp2p:master Apr 23, 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.

refactor gossipsub: make modules private
4 participants