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

misc: make RwStreamSink an implementation detail of the memory transport #4345

Open
thomaseizinger opened this issue Aug 18, 2023 · 4 comments
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted priority:nicetohave

Comments

@thomaseizinger
Copy link
Contributor

The crate rw-stream-sink is a top-level crate in our misc/ directory but it is only in use in two places:

  • For the implementation of the memory transport
  • Within a test in multistream-select

To lower the maintenance effort of the repository, I'd like this crate to be inlined into the memory-transport implementation, ideally as an implementation detail (i.e. not part of the public API).

rw-stream-sink is still published on crates.io, so my suggestion would be to simply depend on it via that for the multistream-select test. This allows us to delete the crate from our repository and move the code into the memory-transport implementation.

@thomaseizinger thomaseizinger added difficulty:easy help wanted decision-pending Marks issues where a decision is pending before we can move forward. labels Aug 18, 2023
@thomaseizinger
Copy link
Contributor Author

@mxinden Let me know if you agree.

@mxinden
Copy link
Member

mxinden commented Aug 31, 2023

I don't have a strong opinion on this. That said, I don't recall this crate causing any significant work. I suggest not investing the time to remove it until such significant work would otherwise be required.

@thomaseizinger
Copy link
Contributor Author

That said, I don't recall this crate causing any significant work. I suggest not investing the time to remove it until such significant work would otherwise be required.

It is true that there is not active workload on the crate. It does cause passive, cognitive complexity because it shows up in the project explorer and in our list of CI jobs.

Thus, I think it is a net positive work item and a great candidate for an external contribution.

@thomaseizinger thomaseizinger added priority:nicetohave getting-started Issues that can be tackled if you don't know the internals of libp2p very well and removed decision-pending Marks issues where a decision is pending before we can move forward. labels Sep 1, 2023
@mxinden
Copy link
Member

mxinden commented Sep 1, 2023

Thus, I think it is a net positive work item and a great candidate for an external contribution.

👍 in favor of removing decision-pending

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted priority:nicetohave
Projects
None yet
Development

No branches or pull requests

2 participants