I should have been clearer. I didn't mean to kill "internal" from stout,
only from Mesos proper (src/ and include/mesos/).
On Tue, Jan 27, 2015 at 8:47 AM, Benjamin Hindman <b...@eecs.berkeley.edu>
wrote:
> I think the point that Jie is making is that we have sometimes wanted to
> have "internal" implementation details exposed in public headers. This is
> common in Stout because everything has to be in headers.
>
> With that in mind, our goal for Mesos should be to have _zero_ internal
> implementation details in public headers. If people find themselves writing
> comments that say "don't use these global variables or functions because
> they're internal to the implementation" then I'll be the first to recommend
> that we reintroduce the 'internal' namespace.
>
> As for Stout I think we should keep the 'internal' namespace.
>
> On Mon, Jan 26, 2015 at 10:25 PM, Jie Yu <yujie....@gmail.com> wrote:
>
> > One benefit of having an internal namespace is that it tells the
> > framework/executor writer that those symbols/method/class are internal to
> > Mesos core and should not be used.
> >
> > If we kill all the internal namespaces and move many headers like
> > isolator.hpp to include/mesos, how does the framework writer know that
> > he/she shouldn't use some of the headers because they are internal to
> Mesos
> > and are subject to change?
> >
> > For modules, I am wondering can we separate Mesos public headers (in
> > include/mesos right now) from those headers that are only for building
> > modules (more like internal public headers).
> >
> > Thoughts?
> >
> > - Jie
> >
> > On Mon, Jan 26, 2015 at 12:35 PM, Kapil Arya <ka...@mesosphere.io>
> wrote:
> >
> > > Hi All,
> > >
> > > TLDR: We currently use "mesos::internal" namespace for almost
> everything
> > > inside src/. However, in most cases, it is directly enclosing another
> > > namespace. This makes the "internal" namespace redundant and we should
> > > kill it.
> > >
> > > I learned from Ben Hindman that the original motivation for introducing
> > an
> > > explicit internal namespace was to discourage people from exposing
> > internal
> > > symbols through "extern", etc. Since we don't seem to expose symbols
> > > through "extern" in our codebase, I think it's safe to kill this
> > namespace.
> > >
> > > Here is a list of files that define classes directly in the
> > mesos::internal
> > > namespace (i.e. without enclosing a separate namespace) [1]:
> > >
> > > authorizer/authorizer.hpp
> > > common/http.hpp
> > > common/attributes.hpp
> > > common/lock.hpp
> > > files/files.hpp
> > > hook/manager.hpp
> > > master/contender.hpp
> > > master/detector.hpp
> > > usage/usage.hpp
> > > watcher/whitelist_watcher.hpp
> > >
> > > messages/messages.pb.h
> > >
> > > Of the above list, things like hook/manager.hpp and
> > > master/{contender,detector}.hpp can be moved into their own namespaces.
> > I
> > > am sure, we can come up with a strategy for the rest as well.
> > >
> > > One possible concern here might be messages.proto and the effects on
> > > upgrades, etc., if we change the namespace/package for these protobufs.
> > If
> > > this turns out to be a real concern, we can possibly keep the internal
> > > namespace for messages.proto.
> > >
> > > If we kill the "internal" namespace altogether, it would make it much
> > > easier to expose some headers as public headers for modularization,
> etc..
> > > This will also help us get rid of "namespace internal" from some of the
> > > public headers that we already have.
> > >
> > > The motivation for killing the "internal" namespace comes from a
> > > patch-set[2] that tries to expose slave/containerizer/isolator.hpp as
> > > include/mesos/slave/isolator.hpp. We wanted to keep Isolator inside
> the
> > > "mesos::slave" namespace instead of putting it directly in the "mesos"
> > > namespace. This change causes a lot of conflicts due to "mesos::slave"
> > and
> > > "mesos::internal::slave" and we had to resort to using fully qualified
> > > names for a large number of definition in src/slave, src/tests, etc.
> > >
> > > Best,
> > > Kapil
> > >
> > > [1] List obtained by running the following command and then filtering
> out
> > > the instances where "internal" was enclosed in something other than
> > > "mesos":
> > > grep -nr "} // namespace internal" * -B1 |grep -v
> > > "namespace\|\-\-\|\.cpp"|cut -d'-' -f1
> > >
> > > [2] https://reviews.apache.org/r/30238/
> > > https://reviews.apache.org/r/29602/
> > > https://reviews.apache.org/r/29603/
> > >
> >
>
