> On Sept. 20, 2014, 7:22 p.m., Ben Mahler wrote: > > Thanks Joris! > > > > Any reason you want this exposed in the public include/ folder of > > libprocess, as opposed to an internal header inside src/? Don't think we'd > > want this in the public includes. > > Niklas Nielsen wrote: > It seems to be a bit unconsistent whether things have gone in include/ or > not. For example, the socket abstraction is only used by libprocess > internally and is in include/. > The node class ended up there too (my / our fault) - so if we decide to > move the private headers to src/, we need to move that alongside doing a scan > for the headers we only use inside libprocess. > > Joris Van Remoortere wrote: > Hi Ben, > I am refactoring certain classes from process.cpp into the include > directory because my goal is to allow pluggable implementations (modules) of > the event-management abstraction. My thought process is that we should have > the classes required to implement this abstraction available in the public > include dir. Since this is an open source project, it's not impossible for us > to have some of these class declarations / definitions in the src directory, > and the module implementer to be required to build it with the src tree > pulled. > I am in the process of making a branch available as a prototype for > MESOS-1330; which will make it more apparent where I am going. > What are your thoughts on the rules behind the public include dir, and > what requirements we can impose on module implementers?
For the public include directory, if the abstraction is generally useful (e.g. Node, Socket is less clearly useful but looks ok) then it's fine to include these in the public headers. `ProcessReference` however, is not something that the library user can leverage, and it's dangerous to expose: use of it can lead to ProcessManager::cleanup spinning forever! > _My thought process is that we should have the classes required to implement > this abstraction available in the public include dir._ Are you interested in applying the same philosophy to the mesos code base? That would grow the public headers substantially. > _Since this is an open source project, it's not impossible for us to have > some of these class declarations / definitions in the src directory, and the > module implementer to be required to build it with the src tree pulled._ Seems like a reasonable thing for module implementers IMHO, otherwise we may have to expose a lot of internals in the public include folder of both libprocess and mesos. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/#review54086 ----------------------------------------------------------- On Sept. 20, 2014, 12:58 a.m., Joris Van Remoortere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25868/ > ----------------------------------------------------------- > > (Updated Sept. 20, 2014, 12:58 a.m.) > > > Review request for mesos and Niklas Nielsen. > > > Repository: mesos-git > > > Description > ------- > > Move class ProcessReference out of process.cpp and into its own header. > Part of refactoring process.cpp. > > > Diffs > ----- > > 3rdparty/libprocess/include/Makefile.am 542ae1c > 3rdparty/libprocess/include/process/process_reference.hpp PRE-CREATION > 3rdparty/libprocess/src/process.cpp 8adc809 > > Diff: https://reviews.apache.org/r/25868/diff/ > > > Testing > ------- > > make check in 3rdparty/libprocess > support/mesos-style.py > > > Thanks, > > Joris Van Remoortere > >
