This sounds reasonable to me. Do others have comments? --- @xujyan <https://twitter.com/xujyan>
On Fri, Jun 23, 2017 at 4:23 PM, James Peach <jor...@gmail.com> wrote: > Hi all, > > There is a common Mesos pattern where a subsystem is implemented by a > facade class that forwards calls to an internal Process class, eg. Fetcher > and FetcherProcess, or zookeeper::Group and zookeeper::GroupProcess. Since > the Process is an internal implementation detail, I'd like to propose that > we adopt a general policy that it should not be exposed in the primary > header file. This has the following benefits: > > - reduces the number of symbols exposed to clients including the primary > header file > - reduces the number of header files needed in the primary header file > - reduces the number of rebuilt dependencies when the process > implementation changes > > Although each individual case of this practice may not improve build > times, I think it is likely that over time, consistent application of this > will help. > > In many cases, when FooProcess is only used by Foo, both the declaration > and definitions of Foo can be inlined into "foo.cpp", which is already our > common practice. If the implementation of the Process class is needed > outside the facade (eg. for testing), the pattern I would propose is: > > foo.hpp - Primary API for Foo, forward declares FooProcess > foo_process.hpp - Declarations for FooProcess > foo_process.cpp - Definitions of FooProcess > > The "checks/checker.hpp" interface almost follows this pattern, but gives > up the build benefits by including "checker_process.hpp" in "checker.hpp". > This should be simple to fix however. > > thanks, > James
