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

Document and clean up marshalling code #50

Open
ryanofsky opened this issue Mar 17, 2021 · 2 comments
Open

Document and clean up marshalling code #50

ryanofsky opened this issue Mar 17, 2021 · 2 comments

Comments

@ryanofsky
Copy link
Collaborator

From bitcoin/bitcoin#19160 (comment), proxy-types marshalling code is hard to read and understand if it's doing the correct thing. I think it can be improved by adding some documentation, also by removing c++11isms (#46), and maybe by making the code less repetitive (same code is pasted repeatedly to deal with raw pointers/shared pointers/unique pointers/optionals, tuples/pairs, map/list/set/vector).

@ariard
Copy link

ariard commented Jul 8, 2021

Eager to help on this issue, to get started what's the fundamental abstraction/types exposed by libmultiprocess ?

We have :

  • Connection
  • EventLoop
  • ProxyClient
  • ProxyServer
  • Proxy Context
  • ?

@ryanofsky
Copy link
Collaborator Author

ryanofsky commented Jul 8, 2021

re: #50 (comment)

This issue is really about cleaning up proxy-types code and adding comments internally. It would be especially great to consolidate BuildField/ReadField overloads with if constexpr and describe how to write custom overloads.

Definitely Connection, EventLoop, etc code could also be cleaned up and documented more, but it is less obvious to me what improvements would be most useful there. I think external code using libmultiprocess can mostly ignore these objects or treat them as opaque. Like if you look at the example code you can see it's declaring an EventLoop and passing it around but only calling 2 methods on it. And there is no reference anywhere in the example code to Connection, ProxyClient, ProxyServer, or ProxyContext. Even bitcoin code is only referencing these things in some kludges. If bitcoin interfaces are cleaned up to have better ownership semantics and no global argsman hacks, bitcoin code doesn't need to use any of these things either. Altnet code should be defining brand new interfaces and also not have to be concerned with these classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants