mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 42662: Added common command utils file.
Date Mon, 25 Jan 2016 22:10:16 GMT


> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote:
> > src/common/command_utils.cpp, lines 153-162
> > <https://reviews.apache.org/r/42662/diff/1/?file=1205717#file1205717line153>
> >
> >     I don't think this check is necessary. We are basically checking what 'tar'
will check later. Can you remove it?
> 
> Jojy Varghese wrote:
>     The only reason I have these validations before we launch the process is to avoid
launching the process if basic validations fail.

That being said, let's try to avoid optimizing (usually means complexity) before there's data
showing that it's a problem.


- Jie


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


On Jan. 22, 2016, 5:38 p.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42662/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2016, 5:38 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This common file is a good place to add common command line utilities like tar,
> digests(sha256, sha512, etc). Currently this functionality is spread in the code
> base and this change would enable all those call sites to be replaced with a
> common code.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/common/command_utils.hpp PRE-CREATION 
>   src/common/command_utils.cpp PRE-CREATION 
>   src/tests/common/command_utils_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42662/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message