> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > include/mesos/mesos.proto, line 94
> > <https://reviews.apache.org/r/4485/diff/2/?file=95884#file95884line94>
> >
> >     You shouldn't link this protobuf explicitly to the downloading of files 
> > before running the executor command. Otherwise if you prefer to do it that 
> > way, embed the message in CommandInfo please.

fixed. dropped it inside CommandInfo


> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > src/common/type_utils.hpp, lines 208-225
> > <https://reviews.apache.org/r/4485/diff/2/?file=95885#file95885line208>
> >
> >     Factor out into operator == for URI please.

done


> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > src/launcher/launcher.cpp, line 274
> > <https://reviews.apache.org/r/4485/diff/2/?file=95891#file95891line274>
> >
> >     Indent.

done


> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > src/launcher/launcher.cpp, line 306
> > <https://reviews.apache.org/r/4485/diff/2/?file=95891#file95891line306>
> >
> >     Space after foreach.

done


> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > src/launcher/launcher.cpp, line 312
> > <https://reviews.apache.org/r/4485/diff/2/?file=95891#file95891line312>
> >
> >     s/r/R

fixed


> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > src/launcher/launcher.cpp, lines 313-315
> > <https://reviews.apache.org/r/4485/diff/2/?file=95891#file95891line313>
> >
> >     strings::trim?

cool..done


> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > src/launcher/main.cpp, line 45
> > <https://reviews.apache.org/r/4485/diff/2/?file=95892#file95892line45>
> >
> >     Factor getenv("MESOS_EXECUTOR_URIS") into a local string uris, then you 
> > won't have to wrap this line.

done


> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > src/launcher/main.cpp, line 47
> > <https://reviews.apache.org/r/4485/diff/2/?file=95892#file95892line47>
> >
> >     s/d/D

done


> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > src/launcher/main.cpp, line 53
> > <https://reviews.apache.org/r/4485/diff/2/?file=95892#file95892line53>
> >
> >     Space around '+'.

done


> On 2012-03-26 20:03:00, Benjamin Hindman wrote:
> > src/launcher/main.cpp, line 44
> > <https://reviews.apache.org/r/4485/diff/2/?file=95892#file95892line44>
> >
> >     Space after foreach.

done


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4485/#review6360
-----------------------------------------------------------


On 2012-03-26 18:39:25, Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4485/
> -----------------------------------------------------------
> 
> (Updated 2012-03-26 18:39:25)
> 
> 
> Review request for mesos, Benjamin Hindman and John Sirois.
> 
> 
> Summary
> -------
> 
>     CommandInfo now accepts a list of URIs.
>     
>     Also added the ability to selectively set the executable
>     bit on the downloaded files.
> 
> 
> This addresses bug mesos-171.
>     https://issues.apache.org/jira/browse/mesos-171
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 23aad17 
>   src/common/type_utils.hpp 557abd7 
>   src/examples/java/TestFramework.java f86646b 
>   src/examples/java/TestMultipleExecutorsFramework.java cdbcc48 
>   src/examples/long_lived_framework.cpp 2775f5a 
>   src/examples/test_framework.cpp 622f6ac 
>   src/launcher/launcher.hpp b48d97c 
>   src/launcher/launcher.cpp 8545193 
>   src/launcher/main.cpp 80567e9 
>   src/slave/http.cpp b9f3232 
>   src/slave/lxc_isolation_module.cpp aff6da6 
>   src/slave/process_based_isolation_module.cpp 4280410 
> 
> Diff: https://reviews.apache.org/r/4485/diff
> 
> 
> Testing
> -------
> 
> make check.
> 
> Yet to test it on a linux machine, as some of the changes affect the lxc code.
> 
> 
> Thanks,
> 
> Vinod
> 
>

Reply via email to