mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 50490: Moved ZooKeeperNetwork into separate hpp and cpp files.
Date Mon, 01 Aug 2016 23:42:52 GMT

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



LGTM, but I'll hold off on an explicit "Ship It!" until I go through the entire chain.

---

For this patch and future patches you make, it would be good to explain the reason behind
the patch inside the commit body (ReviewBoard's "description" field).
For this one, I'd suggest something along the lines of:

```
Separates out the Zookeeper implementation of the replicated log's 
network abstraction.  This is a step towards modularizing the network
abstraction alongside the `MasterDetector` and `MasterContender`.

Note: The network abstraction is a component that discovers all masters
in the group; whereas the `MasterDetector` only discovers the leader
of the group.  Both are necessary for the replicated log.
```

Please do something similar for the rest of the patches in this chain too :)


src/log/log.hpp (lines 22 - 23)
<https://reviews.apache.org/r/50490/#comment210418>

    Newline between these two lines.



src/log/log.cpp (lines 43 - 44)
<https://reviews.apache.org/r/50490/#comment210419>

    Newline between these two lines.


- Joseph Wu


On July 27, 2016, 10:18 p.m., Jay Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50490/
> -----------------------------------------------------------
> 
> (Updated July 27, 2016, 10:18 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5828
>     https://issues.apache.org/jira/browse/MESOS-5828
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Moved ZooKeeperNetwork declaration into log/zookeeper/network.hpp
> and definition into log/zookeeper/network.cpp.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 3d33ee722543c67d0aa72e1c72c578066832331a 
>   src/log/log.hpp a600025f4ca38e3fad0f64f48b007138da8e22d4 
>   src/log/log.cpp f8e439fc756af05acb40878a9473c93c039c889e 
>   src/log/network.hpp 8750880e6cecb6edf4efab4f2b9afe3d8a4d5964 
>   src/log/recover.cpp 69b5b28e5e680aa65a2166b06d3bdbd10c390903 
>   src/log/zookeeper/network.hpp PRE-CREATION 
>   src/log/zookeeper/network.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50490/diff/
> 
> 
> Testing
> -------
> 
> see the end of review chain
> 
> 
> Thanks,
> 
> Jay Guo
> 
>


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