search

haskell-distributed / distributed-process-platform

DEPRECATED (Cloud Haskell Platform) in favor of distributed-process-extras, distributed-process-async, distributed-process-client-server, distributed-process-registry, distributed-process-supervisor, distributed-process-task and distributed-process-execution
http://haskell-distributed.github.com
BSD 3-Clause "New" or "Revised" License
47 stars 18 forks source link

move exported types from C.D.P.P.Supervisor to .Types #80

Closed tavisrudd closed 10 years ago

tavisrudd commented 10 years ago

This is in preparation for some changes to the handling of the StarterProcess variants of ToChildStart. See discussion on PR #77.

hyperthunk commented 10 years ago

Thanks @tavisrudd - will try and get this merged asap. I'm not sure about exposing Supervisor.Types as a public top level module though. Is there really a need for third parties to know about all those Req and Respone types? As an alternative, if we do wish to just export everything and let people figure things out themselves, we could rename it to Supervisor.Internal.Types, but I feel that is of dubious merit if some of the types are meant to be public but some are not. We don't want people importing things from an Internal namespace that they need to use to get on with their day jobs! Of course, they can import the proper public types from Supervisor itself, but then it seems confusing to get both public and private types from something Internal. This was why on that other ticket, I suggested that the idea of just exporting everything inside an "Internal" namespace is not as simple as it necessarily sounds.

Splitting Supervisor.Types up into two bits, public vs private, seems unnecessarily complicated though - I'd recommend keeping the private things private. I don't see how any kind of extension points could be utilised by someone having access to supervisor's internal types here, so I think it's better to just keep private things hidden altogether in this case.

Thoughts?

tavisrudd commented 10 years ago

I agree. I only added it to the exports as an afterthought when one of the tests failed because of it not being exported. I'll look into that failing test and remove it from the exposed modules.

Btw, this new module only exports the types that were previously public in Supervisor. I left the Request types private in Supervisor for now but did consider adding an additional Internal.Types. Only the public Result types are in here.

tavisrudd commented 10 years ago

@hyperthunk it's on other-modules now, where I should have stuck it in the first place, but see my note above re it only containing public types.

hyperthunk commented 10 years ago

Cool thanks @tavisrudd - I'll planning to go through outstanding PRs this weekend.