> On 2012-05-03 20:01:11, Vinod Kone wrote:
> > src/state/state.hpp, line 39
> > <https://reviews.apache.org/r/4981/diff/1/?file=106104#file106104line39>
> >
> >     s/declarations/declaration

There are multiple now.


> On 2012-05-03 20:01:11, Vinod Kone wrote:
> > src/state/state.hpp, line 210
> > <https://reviews.apache.org/r/4981/diff/1/?file=106104#file106104line210>
> >
> >     ws

Done.


> On 2012-05-03 20:01:11, Vinod Kone wrote:
> > src/state/state.cpp, line 59
> > <https://reviews.apache.org/r/4981/diff/1/?file=106105#file106105line59>
> >
> >     why not wrap this in if(db != NULL) then?

No need, 'delete NULL;' is perfectly legal C++, i'm just making a comment here.


> On 2012-05-03 20:01:11, Vinod Kone wrote:
> > src/tests/state_tests.cpp, line 59
> > <https://reviews.apache.org/r/4981/diff/1/?file=106106#file106106line59>
> >
> >     s/ASSERT/EXPECT
> >     
> >     because you always to do the rmdir() irrespective of your  test 
> > succcess/failure

Used test fixtures instead (since this really needs to be an ASSERT).


- Benjamin


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


On 2012-05-04 00:20:14, Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4981/
> -----------------------------------------------------------
> 
> (Updated 2012-05-04 00:20:14)
> 
> 
> Review request for mesos, John Sirois and Vinod Kone.
> 
> 
> Summary
> -------
> 
> This is the beginning of a the components for saving "state" information, 
> including what slaves are connected, what frameworks are running, etc. I'll 
> be adding a ZooKeeperState implementation soon, and the master and it's 
> components will use it to save state in a distributed way.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am cd503a8 
>   src/messages/state.hpp PRE-CREATION 
>   src/messages/state.proto PRE-CREATION 
>   src/state/leveldb.cpp PRE-CREATION 
>   src/state/state.hpp PRE-CREATION 
>   src/state/zookeeper.cpp PRE-CREATION 
>   src/tests/state_tests.cpp PRE-CREATION 
>   src/zookeeper/group.cpp da2e147 
> 
> Diff: https://reviews.apache.org/r/4981/diff
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin
> 
>

Reply via email to