search

sociomantic-tsunami / swarm

Asynchronous client/node framework library
Boost Software License 1.0
14 stars 26 forks source link

Add public MessageParser alias to RequestOnConnBase.EventDispatcher #404

Closed joseph-wakeling-frequenz closed 4 years ago

joseph-wakeling-frequenz commented 4 years ago

The MessageParser symbol is expected to be publicly available via the EventDispatcher class by some downstream code (e.g. RequestHandler in dmqproto), but this is only possible due to implicitly public imports that will stop working for D frontend versions 2.087.0 and later.

Making it available via a public alias both fixes the short-term problem and also allows us an easy route to deprecate this usage in future if we wish to do so.

joseph-wakeling-frequenz commented 4 years ago

This probably needs release notes as well, I'll add shortly.

joseph-wakeling-frequenz commented 4 years ago

Release notes added. This patch removes a blocker to getting dmqnode to build with more recent DMD frontends.

I'm in two minds whether we should take this route, or to update dmqproto, as it's only dmqproto that relies on the implicit import. On the one hand this allows existing downstream use to continue as-is. On the other hand it introduces a new symbol that is likely to be deprecated in future.

joseph-wakeling-frequenz commented 4 years ago

The one use case that I can find is here: https://github.com/sociomantic-tsunami/dmqproto/blob/b3bc6b9564b9fec74dc4deb14708def353620d88/src/dmqproto/node/neo/request/core/RequestHandler.d#L55

... so on balance I think it may be best to just to fix that to import MessageHandler directly, for the time being. If there ever does become something important about accessing MessageHandler via this route, then we can revisit this another time.

joseph-wakeling-frequenz commented 4 years ago

Closing, as I think this is a wrong-headed approach, but I am happy to re-open if anyone thinks differently.