I agree, I'll not export the constructor of DaemonHandle. I'm not sure about not exporting ConfigState. It is meant to be used in a yet unincluded module ConfigWriter, which will take care of manipulating the configuration, saving it to the file etc., so it needs access to everything that ConfigState carries. Ideally, ConfigState should be defined in ConfigWriter, where it'll be the only place it's used, but since the module needs WConfdMonad, that'd create a cyclic dependency between the two modules. I see the following options:
1. Export the whole data type as it is now, knowing that it shouldn't be used anywhere else than in ConfigWriter. 2. Export functions for both reading and modifying the content of ConfigState. This would allow to change the structure of the type without affecting code that uses it. Ideally I'd like to use some kind of functional lenses for it. 3. Add another module for all pure code that is related to the configuration management. This module would include ConfigState and would be imported by Monad as well as ConfigWriter. This would be a good separation, but it doesn't fully solve the problem- the new module would still have to export the internals of ConfigState somehow so that they'd be accessible in ConfigWriter. What would you prefer or suggest? On Thu, Feb 13, 2014 at 5:50 PM, Klaus Aehlig <[email protected]> wrote: > > > +module Ganeti.WConfd.Monad > > + ( ConfigState(..) > > Please only export the type, but not the constructors > > > + , DaemonHandle(..) > > ...and here as well (functions like mkDaemonHandle can be > used outside the module). > > > + , mkDaemonHandle > > + , ClientState(..) > > + , WConfdMonadInt > > + , runWConfdMonadInt > > + , WConfdMonad > > + ) where > > + > > Rest LGTM. Thanks. > > -- > Klaus Aehlig > Google Germany GmbH, Dienerstr. 12, 80331 Muenchen > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores >
