> On Jan. 20, 2015, 1:59 p.m., Kapil Arya wrote: > > src/slave/monitor/monitor.hpp, lines 19-20 > > <https://reviews.apache.org/r/30090/diff/1/?file=827306#file827306line19> > > > > Should we start using `#pragma once` instead? I have been using that in > > a different patchset.
There is still not consensus for that change yet. Let's reopen if that changes before this lands. > On Jan. 20, 2015, 1:59 p.m., Kapil Arya wrote: > > src/slave/monitor/monitor.cpp, lines 21-23 > > <https://reviews.apache.org/r/30090/diff/1/?file=827307#file827307line21> > > > > Rearrange as: > > ``` > > #include "slave/flags.hpp" > > > > #include "slave/monitor/monitor.hpp" > > > > #include "slave/monitor/mesos/monitor.hpp" > > ``` > > > > Since slave/monitor is a subdirectory and so is slave/monitor/mesos We don't have consistent rules around spacing header includes with regards to subfolders. See for example: https://github.com/apache/mesos/blob/master/src/slave/slave.hpp#L50 I'd be happy to file a JIRA and get it in the style guide. Dropping for now. > On Jan. 20, 2015, 1:59 p.m., Kapil Arya wrote: > > src/slave/monitor/monitor.cpp, line 45 > > <https://reviews.apache.org/r/30090/diff/1/?file=827307#file827307line45> > > > > Kill this newline? Good point! Thanks :) > On Jan. 20, 2015, 1:59 p.m., Kapil Arya wrote: > > src/slave/monitor/monitor.cpp, line 56 > > <https://reviews.apache.org/r/30090/diff/1/?file=827307#file827307line56> > > > > Since it's already a `Try<>` type, can't we just do a `return monitor;` > > here? Good catch - yup, can just return monitor. - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30090/#review68786 ----------------------------------------------------------- On Jan. 20, 2015, 1:38 p.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30090/ > ----------------------------------------------------------- > > (Updated Jan. 20, 2015, 1:38 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-2219 > https://issues.apache.org/jira/browse/MESOS-2219 > > > Repository: mesos-git > > > Description > ------- > > Added abstract interface for already existing monitor API. > > > Diffs > ----- > > src/slave/monitor/monitor.hpp PRE-CREATION > src/slave/monitor/monitor.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/30090/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Niklas Nielsen > >
