I'll try and keep my review board titles a little shorter in the future as
well to help prevent this.

Cody

On Tue, Oct 28, 2014 at 3:05 PM, Tim St Clair <tstcl...@redhat.com> wrote:

> My apologies here.
> I had ran support/apply-review.sh and had not trim'd the commit message.
>
> -Tim
>
> ----- Original Message -----
> > From: "Benjamin Mahler" <benjamin.mah...@gmail.com>
> > To: "dev" <dev@mesos.apache.org>
> > Cc: comm...@mesos.apache.org
> > Sent: Tuesday, October 28, 2014 4:38:49 PM
> > Subject: Re: git commit: Add --enable-debug and --enable-optimize flag
> for controlling building debug and optimized
> > verisons of libprocess
> >
> > We try to keep the commit messages short:
> >
> > *5) Follow the format of commit messages. The three important bits are
> (a)
> > be clear and explicit in the commit message and (b) include the link to
> the
> > review and © use 72 character columns. See support/apply-review.sh for
> > committing someone else’s code (it will construct a commit message that
> > you’ll still need to edit because it pulls in all of the ‘Description’
> > which might just be ‘See summary.’ which can be omitted). Note that we
> > don’t always have a 50 character or less summary because that restriction
> > tends to cause people to write poorly.*
> >
> > http://mesos.apache.org/documentation/latest/committers-guide/
> >
> > Check out the commit log to see how this commit message is much longer
> than
> > the others:
> >
> > * 743bb59 (HEAD, apache/master, master) Add --enable-debug and
> > --enable-optimize flag for controlling building debug and optimized
> > verisons of libprocess
> > * 3fc81ce Correct include of linux/ns.hpp in isolator tests.
> > * fa44b0a Use pid namespace to destroy container when available.
> > * 823b992 Correctly recover pid in Linux launcher.
> > * 7b196d2 Pid namespace isolator for the MesosContainerizer.
> > * 691510a Remove Linux namespace functions from stout.
> > * 0342113 Add ns::pid::destroy() to destroy a pid namespace.
> > * 3650573 Add getns() for namespaces.
> > * 57447a7 Move Linux namespace functions into linux/.
> > * 2b8ad0b Include changes to isolation flag when creating Mesos
> > containerizer.
> > * 26824f8 Add Groupon and LIFX to PoweredByMesos documentation.
> > * b2d8df7 Added support for both 1.8 and earlier versions of svn library.
> > * 17ecc0c Added support for module-specific command-line parameters.
> > * 1ff259e Added --isolation flag for tests.
> > * 360e432 Add Artirix to PoweredByMesos list.
> > * f511395 Introduce a shared filesytem isolator.
> > * c18a50a Remove /proc and /sys remounts from port_mapping isolator.
> > * 47fa5a1 Serialize isolator prepare and cleanup (reversed).
> > * 8e6e36a Pass executor directory to Isolator::prepare().
> > * b493875 Adds Localsensor to PoweredByMesos documentation.
> > * fc67600 Fixed line wrap in state/log.cpp.
> > * d418c17 Added --with-apr and --with-svn to Mesos configure.
> > * 7a1020e Added --with-apr and --with-svn to libprocess configure.
> > * 6a778df Updated svn::diff/patch to use newer versions of functions.
> > * fc6f59e Ensured post-reviews.py added newline between subject and body.
> > * 33e625f Added DIFF to the replicated log state storage implementation.
> > * 3b3d60f Added functionality to create SVN based diffs of arbitrary
> > strings.
> > * 61ce00f Added Java replicated log implementation of State.
> > * 1f66bb2 Adds Wizcorp to PoweredByMesos page.
> > * 4bbf727 Reordered functions in type_utils and added an equal comparator
> > for Volume.
> > * 1beacb8 Added documentation for egress rate limit control.
> > * 6ffe580 Added a check in routing library due to a bug in libnl.
> > * b4528ce PortMappingIsolator: Swap TX/RX in statistic collection in
> usage
> > * 6a045e9 Introduced a FutureResult action.
> > * d9381b0 Fixed a spelling mistake.
> > * 12e0674 Mark running tasks killed during framework shutdown.
> >
> > On Tue, Oct 28, 2014 at 2:30 PM, <tstcl...@apache.org> wrote:
> >
> > > Repository: mesos
> > > Updated Branches:
> > >   refs/heads/master 3fc81ce92 -> 743bb5941
> > >
> > >
> > > Add --enable-debug and --enable-optimize flag for controlling building
> > > debug and optimized verisons of libprocess
> > >
> > > Reworks buiding mesos in a "debug" vs. a "release" configuration. By
> > > default, mesos is built in a developer-centric setup (No optimizations,
> > > minimal debug info), in order to maximize developer productivity
> > >
> > > None: '-O0 -g1'
> > > --enable-optimize == '-O2'
> > > --enable-debug == '-g'
> > > --enable-optimize --enable-debug == '-O2 -g'
> > >
> > > If a user / developer passes CXXFLAGS or CFLAGS manually, then they are
> > > not changed / touched at all. This is important so that Mesos is a good
> > > citizen when being built for various distributions (As well as making
> it so
> > > specialized one-off groupings of flags are feasible to use).
> > >
> > > Adds two defines for accessing what mode things are being built in:
> > > 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra
> > > logging and the like. For release builds we may want to set 'NDEBUG'
> which
> > > removes assert()'s, but that is a seperate discussion.
> > >
> > > Review: https://reviews.apache.org/r/27252
> > >
> > >
> > > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> > > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/743bb594
> > > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/743bb594
> > > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/743bb594
> > >
> > > Branch: refs/heads/master
> > > Commit: 743bb5941dd1b3549c76757d939a405ccaa0f279
> > > Parents: 3fc81ce
> > > Author: Cody Maloney <c...@mesosphere.io>
> > > Authored: Tue Oct 28 15:55:40 2014 -0500
> > > Committer: Timothy St. Clair <tstcl...@redhat.com>
> > > Committed: Tue Oct 28 16:30:08 2014 -0500
> > >
> > > ----------------------------------------------------------------------
> > >  3rdparty/libprocess/configure.ac | 47
> +++++++++++++++++++++++------------
> > >  1 file changed, 31 insertions(+), 16 deletions(-)
> > > ----------------------------------------------------------------------
> > >
> > >
> > >
> > >
> http://git-wip-us.apache.org/repos/asf/mesos/blob/743bb594/3rdparty/libprocess/configure.ac
> > > ----------------------------------------------------------------------
> > > diff --git a/3rdparty/libprocess/configure.ac b/3rdparty/libprocess/
> > > configure.ac
> > > index ec4d5a5..18eb681 100644
> > > --- a/3rdparty/libprocess/configure.ac
> > > +++ b/3rdparty/libprocess/configure.ac
> > > @@ -28,14 +28,6 @@ LT_INIT
> > >  LT_LANG([C++])
> > >  LT_OUTPUT
> > >
> > > -# The default CFLAGS/CXXFLAGS from autoconf when using gcc usually
> > > -# includes "-O2". These really slow down compiling our tests, so we
> > > -# turn them off and enable them (where desired) directly in the
> > > -# Makefile. Note that this should not have an impact on users setting
> > > -# CFLAGS/CXXFLAGS directly at configure time, or when running make.
> > > -AS_IF([test "x${ac_cv_env_CFLAGS_set}" = "x"], [CFLAGS="-g"])
> > > -AS_IF([test "x${ac_cv_env_CXXFLAGS_set}" = "x"], [CXXFLAGS="-g"])
> > > -
> > >  # Save the configure arguments so we can pass them to any third-party
> > >  # libraries that we might run configure on (see
> > >  # 3rdparty/Makefile.am). One downside of our strategy for shipping
> > > @@ -56,10 +48,17 @@ AC_ARG_ENABLE([install],
> > >                               [install libprocess]),
> > >                [AC_MSG_ERROR([libprocess can not currently be
> installed])])
> > >
> > > +AC_ARG_ENABLE([debug],
> > > +              AS_HELP_STRING([--enable-debug],
> > > +                             [enable debugging. If CFLAGS/CXXFLAGS are
> > > set, this
> > > +                              option won't change them default: no]),
> > > +              [enable_debug=yes], [])
> > > +
> > >  AC_ARG_ENABLE([optimize],
> > > -              AS_HELP_STRING([--disable-optimize],
> > > -                             [don't try to compile with
> optimizations]),
> > > -              [], [enable_optimize=yes])
> > > +              AS_HELP_STRING([--enable-optimize],
> > > +                             [enable optimizations. If CFLAGS/CXXFLAGS
> > > are set,
> > > +                              this option won't change them default:
> no]),
> > > +              [enable_optimize=yes], [])
> > >
> > >  AC_ARG_ENABLE([perftools],
> > >                AS_HELP_STRING([--enable-perftools],
> > > @@ -525,14 +524,30 @@ AC_SUBST([PROTOBUF_JAR])
> > >  AC_PROG_CXX([g++])
> > >  AC_PROG_CC([gcc])
> > >
> > > -# Check if we should try and enable optimizations.
> > > +# Check if we should enable debugging, optimization. Note we only
> > > +# update CFLAGS and CXXFLAGS if none are provided.
> > > +AM_CONDITIONAL([DEBUG], [test x"$enable_debug" = "xyes"])
> > > +AM_CONDITIONAL([OPTIMIZE], [test x"$enable_optimize" = "xyes"])
> > > +
> > > +
> > > +debug_flags="-g1"
> > > +if test "x$enable_debug" = "xyes"; then
> > > +  debug_flags="-g"
> > > +elif test "x$enable_optimize" = "xyes"; then
> > > +  debug_flags=""
> > > +fi
> > > +
> > >  if test "x$enable_optimize" = "xyes"; then
> > > -  # For now, we only turn on optimizations for gcc.
> > > -  if test "x$GCC" = "xyes"; then
> > > -    CXXFLAGS="$CXXFLAGS -g2 -O2"
> > > -  fi
> > > +  optimize_flags="-O2"
> > > +else
> > > +  optimize_flags="-O0"
> > >  fi
> > >
> > > +AS_IF([test "x${ac_cv_env_CFLAGS_set}" = "x"],
> > > +      [CFLAGS="$debug_flags $optimize_flags"])
> > > +AS_IF([test "x${ac_cv_env_CXXFLAGS_set}" = "x"],
> > > +      [CXXFLAGS="$debug_flags $optimize_flags"])
> > > +
> > >
> > >  # Check if clang was provided instead.
> > >  AC_MSG_CHECKING([if compiling with clang])
> > >
> > >
> >
>
> --
> Cheers,
> Timothy St. Clair
> Red Hat Inc.
>

Reply via email to