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

Re-export all types inside the network module #518

Closed
wants to merge 1 commit into from

Conversation

stevenroose
Copy link
Collaborator

It's really quite confusing which network messages are in which submodules etc. I've been doing some work on rust-bitcoin-p2p over the weekend and this re-export would really simplify things a lot!

pub use self::message::*;
pub use self::message_blockdata::*;
pub use self::message_network::*;
pub use self::message_filter::*;
Copy link
Member

Choose a reason for hiding this comment

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

Can you make an explicit list?

It will be long but using * makes the codebase ungreppable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm concerned about types being forgotten when explicit listing. How do you(/we) feel about making the submodules private so that all types have to be exported (otherwise cargo will shout unused).

@stevenroose
Copy link
Collaborator Author

Ok so I redid this PR, now the internal modules are made private so this is a breaking change. I think the API is a lot more usable like this.

@stevenroose stevenroose added the API break This PR requires a version bump for the next release label Nov 26, 2020
Also make the inner modules private so that all network-related
types come in a single namespace in the API. This gives more ease-of-use
and gives us more flexibility to organize the types as we like.
@stevenroose
Copy link
Collaborator Author

I suppose #525 supersedes this one.

@apoelstra
Copy link
Member

Closing in favor of #525

@apoelstra apoelstra closed this Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API break This PR requires a version bump for the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants