> 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? > > Ben Mahler wrote: > 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.
Thanks for your feedback Ben. I'm fine with requiring module implementers to pull the src; I just did not want to force the issue without a discussion. If everyone else is ok with this then I will keep things in the src dirs. - Joris ----------------------------------------------------------- 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 > >
