Re: Review Request 35089: Replace lock_guard with synchronized in fetcher_cache_test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35089/#review87813 --- Ship it! Ship It! - Benjamin Hindman On June 4, 2015, 10:53 p.m

Re: Review Request 35088: Add dependent include to synchronized.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35088/#review87812 --- Ship it! Ship It! - Benjamin Hindman On June 4, 2015, 10:53 p.m

Re: Review Request 35395: Improvements to Synchronized.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35395/#review87811 --- Ship it! Ship It! - Benjamin Hindman On June 13, 2015, 12:51

Re: Review Request 35423: libprocess: Rebased for _CheckFatal changes in stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35423/#review87808 --- Ship it! Ship It! - Benjamin Hindman On June 13, 2015, 12:24

Re: Review Request 35422: stout: Introduce CHECK_NONE and CHECK_ERROR.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35422/#review87807 --- Ship it! Ship It! - Benjamin Hindman On June 13, 2015, 12:24

Re: Review Request 35405: Enable deleting MasterDetector in MesosSchedulerDriver::join.

://reviews.apache.org/r/35405/diff/ Testing --- NOT YET Thanks, Benjamin Hindman

Re: Review Request 33208: Delete detector in MesosSchedulerDriver::stop

--- On April 15, 2015, 5:23 a.m., Robert Lacroix wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33208/ > ---

Review Request 35405: Enable deleting MasterDetector in MesosSchedulerDriver::join.

--- NOT YET Thanks, Benjamin Hindman

Re: Review Request 29406: Introduce libevent ssl socket.

-- > > (Updated June 12, 2015, 6:06 p.m.) > > > Review request for mesos, Benjamin Hindman and Michael Park. > > > Bugs: MESOS-1913 > https://issues.apache.org/jira/browse/MESOS-1913 > > > Repository: mesos > > > Descr

Re: Review Request 34943: Added validation to flags.

ew86981 ------- On June 11, 2015, 1:52 p.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34943

Re: Review Request 34943: Added validation to flags.

93fd4a5d11d2ae242 src/tests/routing_tests.cpp 4be9967d38177d9f7def5c0da98d4c4266e7f0b5 src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 Diff: https://reviews.apache.org/r/34943/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

> On June 5, 2015, 6:42 p.m., Vinod Kone wrote: > > I made some minor comments below but I think a better way to do this is to > > *not* write custom masterinfo json <-> protobuf converters. I prefer we > > just add a new optional field (say ipAddress of type string). Then you can > > just lev

Re: Review Request 35207: Included doxygen documentation in docs/home.md.

207/#comment139468> This looks like it's the same thing that Niklas added in https://reviews.apache.org/r/35037? And this link doesn't work, 'documentation' is spelled wrong. I'm just going to kill this part for now since we're already linking to this from what Nik

Re: Review Request 35206: Added mainpage to doxygen documentation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35206/#review87153 --- Ship it! Ship It! - Benjamin Hindman On June 8, 2015, 8:51 p.m

Re: Review Request 34644: Update existing lambdas to meet style guide

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34644/#review87150 --- Ship it! Ship It! - Benjamin Hindman On June 1, 2015, 5:45 p.m

Re: Review Request 34645: Update existing lambdas to meet style guide

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34645/#review87149 --- Ship it! Ship It! - Benjamin Hindman On June 1, 2015, 5:26 p.m

Re: Review Request 34944: Used flags validation to handle --help.

generated e-mail. To reply, visit: https://reviews.apache.org/r/34944/#review86231 ------- On June 2, 2015, 2:46 p.m., Benjamin Hindman wrote: > > --- > This

Re: Review Request 34943: Added validation to flags.

ith hazy about the new { } initializer, but > > could have this been: > > ``` > > char* argv[] = { (char*) "/path/to/program", > > (char*) "--name1=billy joel" }; > > ``` > > Benjamin Hindman wrote: > I just c

Re: Review Request 34943: Added validation to flags.

he.org/r/34943/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 34943: Added validation to flags.

34943/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 35125: Fixed verb naming of Flag::loader to Flag::load.

: https://reviews.apache.org/r/35125/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 34943: Added validation to flags.

(char*) "--name1=billy joel" }; > > ``` I just copied this code. Let me give this a test and if so I'll cleanup the others too. - Benjamin --- This is an automatically generated e-mail. To re

Review Request 35127: Updated libprocess to match stout Flags changes.

--- See summary. Diffs - 3rdparty/libprocess/src/subprocess.cpp f41f5e2a34788e31749eb996c8ab38ea45989068 Diff: https://reviews.apache.org/r/35127/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 35125: Fixed verb naming of Flag::loader to Flag::load.

/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 35128: Updated Mesos to match stout Flags changes.

--- See summary. Diffs - src/master/http.cpp f8ac30934352db859e73819e0656a70047bb0dc5 src/slave/http.cpp bc25bdd33277dbfa30410ad081ea09f0fc39c598 Diff: https://reviews.apache.org/r/35128/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 35126: Used C++11 lambdas instead of functors in FlagsBase.

/libprocess/3rdparty/stout/tests/flags_tests.cpp 80450185f60c5b273face490e0bb9e695b0cb984 3rdparty/libprocess/3rdparty/stout/tests/subcommand_tests.cpp c40bba4f1e7eef7cb04f79b567e32684648b2004 Diff: https://reviews.apache.org/r/35126/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 33159: Pump updateFramework through Allocator from Master.

tps://reviews.apache.org/r/33159/#comment138859> CHECK_EQ - Benjamin Hindman On June 5, 2015, 8:53 a.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 33850: libprocess: use standard macros for compiler and vendor detection

g/r/33850/#comment138676> I think you meant != here. ;-) How did this pass for you!? - Benjamin Hindman On May 15, 2015, 4:24 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 33849: mesos: use standard macros for compiler and vendor detection

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33849/#review86617 --- Ship it! Ship It! - Benjamin Hindman On May 5, 2015, 5:52 p.m

Re: Review Request 33849: mesos: use standard macros for compiler and vendor detection

reviews.apache.org/r/33849/#review82524 --- On May 5, 2015, 5:52 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33849/ > ---

Re: Review Request 33849: mesos: use standard macros for compiler and vendor detection

> On June 4, 2015, 2:44 p.m., Benjamin Hindman wrote: > > Ship It! Like another patch you submitted you went from +2 to +4 indentation here, please be on the look out for that in the future. Thanks! - Benjamin --- T

Re: Review Request 33755: Remove the unused acx_pthread autoconf macro from stout.

> On June 4, 2015, 1:47 p.m., Benjamin Hindman wrote: > > Ship It! Note that I also had to update stout/Makefile.am to remove the usage of the 'm4' directory. I'm not sure how this didn't break running 

Re: Review Request 33850: libprocess: use standard macros for compiler and vendor detection

> On June 4, 2015, 2:04 p.m., Benjamin Hindman wrote: > > Just a heads up, you went from the +2 indentation in this file to +4 when you moved stuff around. I fixed it for you, but please keep the style consistent in the future, thanks! -

Re: Review Request 33850: libprocess: use standard macros for compiler and vendor detection

> On June 4, 2015, 2:04 p.m., Benjamin Hindman wrote: > > 3rdparty/libprocess/configure.ac, line 694 > > <https://reviews.apache.org/r/33850/diff/2/?file=950418#file950418line694> > > > > s/Mesos/libprocess/ I took care of this fix for you James before

Re: Review Request 33850: libprocess: use standard macros for compiler and vendor detection

tps://reviews.apache.org/r/33850/#comment138661> s/Mesos/libprocess/ - Benjamin Hindman On May 15, 2015, 4:24 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 33753: Update pthread and python autoconf macros for Mesos.

> On May 18, 2015, 6:20 p.m., haosdent huang wrote: > > configure.ac, line 650 > > > > > > Should we change acx_pthread.m4 to ax_pthread.m4 here? > > James Peach wrote: > Updated the patch to alter the comments to

Re: Review Request 33754: Update pthread autoconf macros for libprocess.

> On May 18, 2015, 6:24 p.m., haosdent huang wrote: > > 3rdparty/libprocess/configure.ac, line 745 > > > > > > Maybe we could search the whole project and replace acx_pthread.m4 -> > > ax_pthread.m4 I fixed this for

Re: Review Request 33753: Update pthread and python autoconf macros for Mesos.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33753/#review86598 --- Ship it! Ship It! - Benjamin Hindman On May 1, 2015, 4:02 p.m

Re: Review Request 33754: Update pthread autoconf macros for libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33754/#review86597 --- Ship it! Ship It! - Benjamin Hindman On May 1, 2015, 4:02 p.m

Re: Review Request 33755: Remove the unused acx_pthread autoconf macro from stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33755/#review86596 --- Ship it! Ship It! - Benjamin Hindman On May 1, 2015, 4:01 p.m

Re: Review Request 35065: Fix uninitialized warning introduced by synchronized patches.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35065/#review86590 --- Ship it! Ship It! - Benjamin Hindman On June 4, 2015, 12:16 p.m

Re: Review Request 35012: Move synchronized.hpp into stout.

/synchronized.hpp <https://reviews.apache.org/r/35012/#comment138627> __STOUT_SYNCHRONIZED_HPP__ - Benjamin Hindman On June 3, 2015, 5:13 p.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 35013: Move synchronized.hpp out of libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35013/#review86572 --- Ship it! Ship It! - Benjamin Hindman On June 3, 2015, 5:14 p.m

Re: Review Request 32365: Remove libprocess internal.hpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32365/#review86571 --- Ship it! Ship It! - Benjamin Hindman On June 3, 2015, 5:19 p.m

Re: Review Request 32358: Refactor Future to use synchronized.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32358/#review86565 --- Ship it! Ship It! - Benjamin Hindman On June 3, 2015, 5:16 p.m

Re: Review Request 32361: Refactor Queue to use synchronized.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32361/#review86567 --- Ship it! Ship It! - Benjamin Hindman On June 3, 2015, 5:16 p.m

Re: Review Request 32356: Refactor synchronized to use mutex, recursive_mutex, atomic_flag.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32356/#review86564 --- Ship it! Ship It! - Benjamin Hindman On June 3, 2015, 5:11 p.m

Re: Review Request 32363: Refactor Metrics::Metric to use synchronized.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32363/#review86569 --- Ship it! Ship It! - Benjamin Hindman On June 3, 2015, 5:18 p.m

Re: Review Request 32364: Refactor http to use synchronized.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32364/#review86570 --- Ship it! Ship It! - Benjamin Hindman On June 3, 2015, 5:19 p.m

Re: Review Request 32362: Refactor Metrics::Timer to use synchronized.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32362/#review86568 --- Ship it! Ship It! - Benjamin Hindman On June 3, 2015, 5:17 p.m

Re: Review Request 32360: Refactor Mutex to use synchronized.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32360/#review86566 --- Ship it! Ship It! - Benjamin Hindman On June 3, 2015, 5:16 p.m

Review Request 35000: Doxygen'ized Subprocess.

Description --- See summary. Diffs - 3rdparty/libprocess/include/process/subprocess.hpp 37cab7755d2890619b64e1ca09e0b7ad0e72cf76 Diff: https://reviews.apache.org/r/35000/diff/ Testing --- make check doxygen ../Doxyfile Thanks, Benjamin Hindman

Re: Review Request 34944: Used flags validation to handle --help.

/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 34944: Used flags validation to handle --help.

Thanks, Benjamin Hindman

Review Request 34943: Added validation to flags.

a6e8ba943d97ae908122a444332155ebc6c7bb93 Diff: https://reviews.apache.org/r/34943/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 33275: Fix capture by reference of temporaries in Stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33275/#review86195 --- Ship it! Ship It! - Benjamin Hindman On May 20, 2015, 10:36 p.m

Re: Review Request 33272: Fix capture by reference of temporary strings in Stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33272/#review86194 --- Ship it! Ship It! - Benjamin Hindman On May 20, 2015, 10:36 p.m

Re: Review Request 33276: Fix capture by reference of temporaries in Libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33276/#review86192 --- Ship it! Ship It! - Benjamin Hindman On May 20, 2015, 10:36 p.m

Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review86191 --- Ship it! Ship It! - Benjamin Hindman On May 20, 2015, 10:36 p.m

Re: Review Request 33271: Update style guide to disallow capturing temporaries by reference.

tps://reviews.apache.org/r/33271/#comment138129> I totally didn't follow this. ;-) How about something as simple as: vector strings{"hello"}; string& s = strings[0]; strings.erase(strings.begin()); s += "world"; // THIS IS A DANGLING REFERENC

Re: Review Request 33271: Update style guide to disallow capturing temporaries by reference.

271/#comment138125> What about this case: T t("Hello")' const T& tprime = t.Member(); - Benjamin Hindman On June 2, 2015, 9:34 a.m., Joris Van Remoortere wrote: > > --- > This is an

Re: Review Request 34563: Allowed delegating constructors in styleguide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34563/#review86183 --- Ship it! Ship It! - Benjamin Hindman On May 21, 2015, 8:55 p.m

Re: Review Request 34565: Allowed explicitly-defaulted functions in styleguide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34565/#review86182 --- Ship it! Ship It! - Benjamin Hindman On May 21, 2015, 8:54 p.m

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

te<>' that shouldn't be there? src/common/parse.hpp <https://reviews.apache.org/r/34687/#comment138108> Operators, including '+' on previous line please. src/tests/common/parse_tests.cpp <https://reviews.apache.org/r/34687/#comment138123> ASSER

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34195/#review86116 --- Ship it! Ship It! - Benjamin Hindman On May 29, 2015, 1:10 a.m

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

> On May 28, 2015, 11:45 a.m., Benjamin Hindman wrote: > > src/cli/execute.cpp, line 321 > > <https://reviews.apache.org/r/34195/diff/3/?file=972213#file972213line321> > > > > I'm not convinced the 'errorMessage' is more help here. You'

Re: Review Request 34892: Reintroduced chown stdout stderr in FetcherProcess::run().

s for things like this that require another user. - Benjamin Hindman On June 1, 2015, 3:42 p.m., Bernd Mathiske wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.a

Re: Review Request 30774: Fetcher Cache

tps://reviews.apache.org/r/30774/#comment137806> Using locks here is dangerous because on a smaller machine depending on the number of requests we have come through a test we might actually deadlock the entire process. We should call this out explicitly and leave a TODO on how we can do t

Re: Review Request 33781: Add license blobs to Java JNI cpp files

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33781/#review85558 --- Ship it! Ship It! - Benjamin Hindman On May 2, 2015, 2:35 p.m

Re: Review Request 34193: Refactored common functionality into FlagsBase

This looks like a duplicated test with the 'Usage' test above? 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp <https://reviews.apache.org/r/34193/#comment137113> s/[ERROR]// - Benjamin Hindman On May 26, 2015, 8:47 p.m., Marco Massenzio wrote: > > -

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

ithin a single process."; src/log/tool/benchmark.cpp <https://reviews.apache.org/r/34195/#comment137128> Why are we not killing this 'usage'? src/log/tool/initialize.cpp <https://reviews.apache.org/r/34195/#comment137129> Why are we not killing this &#

Re: Review Request 34654: Send docker inspect output with TaskStatus data.

6938> You should be able to drop the parameter all together, in the lambdas below as well. Can you give that a shot and let me know if it doesn't work please? src/docker/executor.cpp <https://reviews.apache.org/r/34654/#comment136937> Can we capture this as a constant plea

Re: Review Request 33271: Update style guide to disallow capturing temporaries by reference.

e it's such a common pattern please? Also, let's show some more explicit alias examples. docs/mesos-c++-style-guide.md <https://reviews.apache.org/r/33271/#comment136724> Why is this a bad alias? - Benjami

Re: Review Request 34193: Refactored common functionality into BaseFlags

we s/BaseFlags/FlagsBase/ everywhere in this review (and Summary, etc). ;-) - Benjamin Hindman On May 20, 2015, 11:22 p.m., Marco Massenzio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://rev

Re: Review Request 34276: Use special constructor for Option from Some.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34276/#review85197 --- Ship it! Ship It! - Benjamin Hindman On May 21, 2015, 11:45 p.m

Re: Review Request 34278: Refactor Stout Result leveraging Try> to remove the dynamic allocation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34278/#review85199 --- Ship it! Ship It! - Benjamin Hindman On May 21, 2015, 11:45 p.m

Re: Review Request 34277: Refactor Stout Try leveraging Option to remove dynamic allocation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34277/#review85198 --- Ship it! Ship It! - Benjamin Hindman On May 21, 2015, 11:45 p.m

Re: Review Request 34193: Refactored common functionality into BaseFlags

review but he just addressed a single place instead of all the places. ;-) Thanks guys! 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp <https://reviews.apache.org/r/34193/#comment136576> With the revised 'usage()' this will just be: out <&l

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

Then let's get an AWAIT_READY_TRUE implemented later! I can help, just let me know! src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/29889/#comment136556> Looks like we need another helper in libprocess! Are you looking for something along the lines of:

Re: Review Request 30606: Added net::contentLength() to query "content-length" field from HTTP header.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30606/#review84855 --- Ship it! Ship It! - Benjamin Hindman On Feb. 28, 2015, 2:51 p.m

Re: Review Request 30609: Added a function that reports file size, not following links.

> On May 22, 2015, 2:31 a.m., Benjamin Hindman wrote: > > These are minor nits so I'll take care of them for you and commit this, thanks Bernd! - Benjamin --- This is an automatically generated e-mail. To reply

Re: Review Request 30609: Added a function that reports file size, not following links.

6> Minor nit but you can remove the constant: ASSERT_SOME(os::write(file, string(fileSize.bytes(), 'X'))); - Benjamin Hindman On May 21, 2015, 4:48 a.m., Bernd Mathiske wrote: > > --- > This is a

Re: Review Request 34442: Support manipulating scheduler affinity on Linux.

That the implementation of sched_setaffinity and sched_setaffinity correctly works? That's not our code so we don't need to bother testing it. src/tests/sched_tests.cpp <https://reviews.apache.org/r/34442/#comment135738> When do we get `sync

Re: Review Request 34018: Update existing lambdas to meet style guide

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34018/#review84222 --- Ship it! Ship It! - Benjamin Hindman On May 17, 2015, 10:11 a.m

Re: Review Request 34017: Update existing lambdas to meet style guide

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34017/#review84221 --- Ship it! Ship It! - Benjamin Hindman On May 17, 2015, 10:11 a.m

Re: Review Request 34371: Add framework's pid to json summary of framework.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34371/#review84220 --- Ship it! Ship It! - Benjamin Hindman On May 18, 2015, 8:57 p.m

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

E(Collection(lines).exists([=](const string& line) { return line == "err" + uuid; }); EXPECT_TRUE(Collection(lines).exists(line)); EXPECT_TRUE(containsLine(lines, "err" + uuid)); Collection(strings::split(read.get())) .exists([](const string&

Re: Review Request 33505: Add state-summary endpoint to master.

.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33505/ > --- > > (Updated May 5, 2015, 9:51 p.m.) &

Re: Review Request 33643: Add EMPTY to stout hashset

- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33643/ > --- > > (Updated May 5, 2015, 9:50 p.m.) > > > Review request for

Re: Review Request 32837: Re-order structs in Slave State.hpp to prevent forward declaration dependency.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32837/#review82328 --- Ship it! Ship It! - Benjamin Hindman On April 30, 2015, 11:43

Re: Review Request 32837: Re-order structs in Slave State.hpp to prevent forward declaration dependency.

> On April 30, 2015, 9:45 p.m., Ben Mahler wrote: > > src/slave/state.hpp, lines 214-227 > > > > > > It seems a bit odd to put the top-level state in the middle, before one > > can see the tree structure through the o

Re: Review Request 33558: Add C++11 lambdas to the C++ style guide.

point to preferring the version that doesn't do it this way. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33558/#review81643 -------

Re: Review Request 33558: Add C++11 lambdas to the C++ style guide.

ke this: > > ```cpp > SomeLongReturnType SomeLongFunctionName( > const T1& p1, const T2& p2, const T3& p3) > { > ... > } > ``` > > By "further" above I'm referring to the fact that we put a newl

Re: Review Request 30774: Fetcher Cache

ernd/ src/tests/fetcher_tests.cpp <https://reviews.apache.org/r/30774/#comment132992> EXPECT_TRUE(strings::contains(fetch.failure(), "chown")); (How about we just search and replace .find?) src/tests/mesos.cpp <https://reviews.apache.org/r/30774/#comme

Re: Review Request 30774: Fetcher Cache

cher fetcher(flags); Slave slave(..., &fetcher); Slave::registered(...) { ...; Try recover = fetcher->recover(slaveid); if (recover.is...) { ...; } ...; } ------ But for now, let's just s/recoverCache/recover/ since the fact that the fetcher has a c

Re: Review Request 30774: Fetcher Cache

static method rather than invoking a method on an instance of a 'Fetcher' directly. Our intuition is that this will likely have to change in the future. src/slave/slave.cpp <https://reviews.apache.org/r/30774/#comment127350> Comment here too, please, t

Re: Review Request 33558: Add C++11 lambdas to the C++ style guide.

-guide.md fe98f90ad0b0f5dd38af97e85062e90cee8de99e Diff: https://reviews.apache.org/r/33558/diff/ Testing --- N/A Thanks, Benjamin Hindman

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

tps://reviews.apache.org/r/33295/#comment132317> namespace process { namespace firewall { void install(std::vector>&& rules); } // namespace firewall { } // namespace process { Also, consider how we can best create a /__firewall__ route. - Benjamin Hind

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

reate = Docker::create(flags.docker); if (create.isError()) { return Error("Failed to create Docker: " + create.error()); } Shared docker(create.get()); - Benjamin Hindman On April 22, 2015, 10:50 p.m., Timothy Chen wrote: > > -

Re: Review Request 33558: Add C++11 lambdas to the C++ style guide.

ink? I know we had already talked about this, but I > > realized this later. Great! I've updated the review. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.

<    2   3   4   5   6   7   8   >