> On Sept. 11, 2012, 7 p.m., Ben Mahler wrote:
> > src/Makefile.am, line 157
> > <https://reviews.apache.org/r/6842/diff/1/?file=147305#file147305line157>
> >
> >     this breaks the line alignment, put it after slave/gc.cpp?

fixed tab alignment


> On Sept. 11, 2012, 7 p.m., Ben Mahler wrote:
> > src/Makefile.am, line 210
> > <https://reviews.apache.org/r/6842/diff/1/?file=147305#file147305line210>
> >
> >     This also doesn't align, looks like someone used tabs for the end of 
> > line \ alignment, where tabs were treated as... 8 spaces!
> >     
> >     I don't see why we didn't just do 1 file per line in our makefiles.. 
> > seems harder for humans to read this way
> >     
> >     anyway, feel free to ignore this :)

fixed with tabs=8 spaces, for now


> On Sept. 11, 2012, 7 p.m., Ben Mahler wrote:
> > src/Makefile.am, line 769
> > <https://reviews.apache.org/r/6842/diff/1/?file=147305#file147305line769>
> >
> >     ditto on the alignment

fixed


> On Sept. 11, 2012, 7 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/format.hpp, line 234
> > <https://reviews.apache.org/r/6842/diff/1/?file=147312#file147312line234>
> >
> >     what happened here?

probably bad diff. can't see it in my file anymore.


> On Sept. 11, 2012, 7 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/os.hpp, line 178
> > <https://reviews.apache.org/r/6842/diff/1/?file=147313#file147313line178>
> >
> >     instead of const_cast, could we just use strcpy into a non-const char* 
> > instead?

this is how we do it everywhere else in this file, so i would like to keep it 
this way. any particular reason you want to change it?


> On Sept. 11, 2012, 7 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/os.hpp, line 378
> > <https://reviews.apache.org/r/6842/diff/1/?file=147313#file147313line378>
> >
> >     ditto

ditto


> On Sept. 11, 2012, 7 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/format.hpp, line 291
> > <https://reviews.apache.org/r/6842/diff/1/?file=147312#file147312line291>
> >
> >     one idea:
> >     
> >     store t as a const&
> >     then in get(), do the stringify(t).c_str()

i wish that worked, but it didnt :/


- Vinod


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


On Aug. 29, 2012, 11:09 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6842/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2012, 11:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> And so it begins...
> 
> I am discarding the old review associated with this (previously called path: 
> https://reviews.apache.org/r/4944/), in favor of a fresh start.
> 
> slave/state.{hpp,cpp} contains all the functions that deal with persistent 
> state of the slave.
> 
> 
> This addresses bug MESOS-110.
>     https://issues.apache.org/jira/browse/MESOS-110
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am aaceee3 
>   src/common/protobuf_utils.hpp PRE-CREATION 
>   src/slave/slave.hpp 10c537b 
>   src/slave/slave.cpp 4efd41e 
>   src/slave/state.hpp PRE-CREATION 
>   src/slave/state.cpp PRE-CREATION 
>   src/tests/slave_state_tests.cpp PRE-CREATION 
>   third_party/libprocess/include/stout/format.hpp c746ad3 
>   third_party/libprocess/include/stout/os.hpp b1eceb3 
> 
> Diff: https://reviews.apache.org/r/6842/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to