> On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/launcher/launcher.cpp, line 262 > > <https://reviews.apache.org/r/4119/diff/1/?file=86909#file86909line262> > > > > Kill newline.
done > On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/launcher/launcher.cpp, line 227 > > <https://reviews.apache.org/r/4119/diff/1/?file=86909#file86909line227> > > > > Again, these fatal errors are before we have forked the executor ... > > this is really a bug on our part. it's forked already > On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/launcher/launcher.cpp, line 218 > > <https://reviews.apache.org/r/4119/diff/1/?file=86909#file86909line218> > > > > We use spaces between 'if' and '('. done > On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/launcher/launcher.cpp, line 219 > > <https://reviews.apache.org/r/4119/diff/1/?file=86909#file86909line219> > > > > Indent +2 only please. done > On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/launcher/launcher.cpp, line 214 > > <https://reviews.apache.org/r/4119/diff/1/?file=86909#file86909line214> > > > > It would be cool to see performing an HTTP request get factored out > > into it's own utility! ;) done > On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/launcher/launcher.cpp, lines 209-210 > > <https://reviews.apache.org/r/4119/diff/1/?file=86909#file86909line209> > > > > You should be able to get away with just a string here. done > On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/launcher/launcher.cpp, line 206 > > <https://reviews.apache.org/r/4119/diff/1/?file=86909#file86909line206> > > > > To be clear, this means the process will be killed. At this point we > > haven't yet forked, so a user could construct an executor URI that kills > > the slave. Maybe this means we should actually have forked by this point? > > Or maybe this means this shouldn't be a CHECK. it's forked already actually (discussed this yesterday) > On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/launcher/launcher.cpp, lines 199-200 > > <https://reviews.apache.org/r/4119/diff/1/?file=86909#file86909line199> > > > > No need to declare these up here, go ahead and put them where they are > > defined. done > On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/launcher/launcher.cpp, line 32 > > <https://reviews.apache.org/r/4119/diff/1/?file=86909#file86909line32> > > > > Throw this one above <sys/...>. We try and organize these in > > alphabetically for each nesting level, first for the .h's, then the C++ > > style headers, then the .hpp's, then the local headers (i.e., the ones in > > quotes). done > On 2012-03-02 20:38:28, Benjamin Hindman wrote: > > src/Makefile.am, line 246 > > <https://reviews.apache.org/r/4119/diff/1/?file=86908#file86908line246> > > > > s/Library for handling/Libraries for performing done - Florian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4119/#review5586 ----------------------------------------------------------- On 2012-03-06 15:58:46, Florian Leibert wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4119/ > ----------------------------------------------------------- > > (Updated 2012-03-06 15:58:46) > > > Review request for mesos and Benjamin Hindman. > > > Summary > ------- > > Launcher can now retrieve an executor specified as a HTTP Url. > Added dependencies of libcurl, ssl, crypto & z to the Makefile. > > > Diffs > ----- > > src/launcher/launcher.cpp 9bbae7e > src/Makefile.am 1d108ab > src/common/utils.hpp 5a88f17 > > Diff: https://reviews.apache.org/r/4119/diff > > > Testing > ------- > > > Thanks, > > Florian > >
