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



src/common/utils.hpp
<https://reviews.apache.org/r/4899/#comment16644>

    touch doesn't truncate - this probably should not either ... or else the 
name should change.



src/common/utils.hpp
<https://reviews.apache.org/r/4899/#comment16642>

    You copied local style as in the inlined open method above, but I suspect 
that was itself a typo based on other inline functions in this file and other 
files in mesos.  -2 indent here and elsewhere on function opening { and closing 
} ?



src/common/utils.hpp
<https://reviews.apache.org/r/4899/#comment16646>

    You own this TODO



src/common/utils.hpp
<https://reviews.apache.org/r/4899/#comment16647>

    SEEK_SET or should the doc be ammended to say read the file from its 
current offset to the end?



src/common/utils.hpp
<https://reviews.apache.org/r/4899/#comment16648>

    This could error as well as the 2 lseeks above, seems like that should be 
handled and propagated via the Try return type.



src/common/utils.hpp
<https://reviews.apache.org/r/4899/#comment16650>

    You should s/pattern/prefix/ - afaict thats all you will successfully match.



src/common/utils.hpp
<https://reviews.apache.org/r/4899/#comment16661>

    Instead of a recursive function, why not leverage ftw and even fnmatch if 
you want some globbiness.


- John


On 2012-05-03 19:23:34, Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4899/
> -----------------------------------------------------------
> 
> (Updated 2012-05-03 19:23:34)
> 
> 
> Review request for mesos and John Sirois.
> 
> 
> Summary
> -------
> 
> We can now read/write strings to a file.
> 
> Some common utils for working with protobuf objects.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am cd503a8 
>   src/common/protobuf_utils.hpp PRE-CREATION 
>   src/common/utils.hpp 1d81e21 
>   src/slave/slave.cpp b233b68 
>   src/tests/protobuf_io_tests.cpp 22f37ac 
>   src/tests/utils_tests.cpp 49ec107 
> 
> Diff: https://reviews.apache.org/r/4899/diff
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod
> 
>

Reply via email to