You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2014/01/06 20:40:33 UTC

Review Request 16662: Refactorings necessary to get Mesos to compile with clang.

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

Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Repository: mesos-git


Description
-------

See summary.


Diffs
-----

  include/mesos/executor.hpp 9b258347c79bf54a85cc64fc84ca7a433d8d847f 
  include/mesos/scheduler.hpp 161cc659bf971a758406d42573a43299327f0b43 
  src/files/files.cpp 45ef95cd40c4cf2d9235668860b032e96cc81066 
  src/master/allocator.hpp 85ed2141b4539f383ad9e2527321e066cb5429f6 
  src/master/contender.cpp 84b0552a1375726d9a2fd6213b49e13f49027c3d 
  src/master/http.cpp d7cd89f0a3446f4c2e65ecd259544149bf92faf8 
  src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4 
  src/master/master.cpp 38c553249a11745ca96279c755da8c36bbb3c053 
  src/master/registrar.cpp 61fdea3a300eabf5037288a6356a04b07c363980 
  src/slave/http.cpp 1358810cde91fc64d190196e0a7675fa30d8f106 
  src/slave/isolator.hpp fc13089664e136936ed30ae4a5a11ab55d7719dc 
  src/slave/slave.hpp b00f97057fd2f9ba72364cb41c543bd24a97b0fa 
  src/slave/slave.cpp 396293b1749afcbebdf366f4105b46e8ec7749de 
  src/slave/status_update_manager.cpp f7a0c40f1b1a8cfab828998146b8f446a0569251 
  src/state/state.hpp 02620b3292849ec7cb85cdf3b1de8bd0796ae597 

Diff: https://reviews.apache.org/r/16662/diff/


Testing
-------

make check


Thanks,

Benjamin Hindman


Re: Review Request 16662: Refactorings necessary to get Mesos to compile with clang.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16662/#review31581
-----------------------------------------------------------

Ship it!



src/master/master.hpp
<https://reviews.apache.org/r/16662/#comment60094>

    In this case, we don't need to create a copy of the Task and we could just use a shared_ptr everywhere for Task.
    
    But this is a good minimal change. So LGTM.


- Ben Mahler


On Jan. 6, 2014, 7:40 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16662/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2014, 7:40 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 9b258347c79bf54a85cc64fc84ca7a433d8d847f 
>   include/mesos/scheduler.hpp 161cc659bf971a758406d42573a43299327f0b43 
>   src/files/files.cpp 45ef95cd40c4cf2d9235668860b032e96cc81066 
>   src/master/allocator.hpp 85ed2141b4539f383ad9e2527321e066cb5429f6 
>   src/master/contender.cpp 84b0552a1375726d9a2fd6213b49e13f49027c3d 
>   src/master/http.cpp d7cd89f0a3446f4c2e65ecd259544149bf92faf8 
>   src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4 
>   src/master/master.cpp 38c553249a11745ca96279c755da8c36bbb3c053 
>   src/master/registrar.cpp 61fdea3a300eabf5037288a6356a04b07c363980 
>   src/slave/http.cpp 1358810cde91fc64d190196e0a7675fa30d8f106 
>   src/slave/isolator.hpp fc13089664e136936ed30ae4a5a11ab55d7719dc 
>   src/slave/slave.hpp b00f97057fd2f9ba72364cb41c543bd24a97b0fa 
>   src/slave/slave.cpp 396293b1749afcbebdf366f4105b46e8ec7749de 
>   src/slave/status_update_manager.cpp f7a0c40f1b1a8cfab828998146b8f446a0569251 
>   src/state/state.hpp 02620b3292849ec7cb85cdf3b1de8bd0796ae597 
> 
> Diff: https://reviews.apache.org/r/16662/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 16662: Refactorings necessary to get Mesos to compile with clang.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Jan. 6, 2014, 10:02 p.m., Niklas Nielsen wrote:
> > src/master/registrar.cpp, lines 211-212
> > <https://reviews.apache.org/r/16662/diff/1/?file=417609#file417609line211>
> >
> >     Wouldn't the usual wrapping and indent be:
> >     
> >     LOG(WARNING) << "Failed to recover registrar: "
> >                  << (recovery.isFailed() ...
> >     ?

Fixed, thanks!


- Benjamin


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


On Jan. 6, 2014, 7:40 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16662/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2014, 7:40 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 9b258347c79bf54a85cc64fc84ca7a433d8d847f 
>   include/mesos/scheduler.hpp 161cc659bf971a758406d42573a43299327f0b43 
>   src/files/files.cpp 45ef95cd40c4cf2d9235668860b032e96cc81066 
>   src/master/allocator.hpp 85ed2141b4539f383ad9e2527321e066cb5429f6 
>   src/master/contender.cpp 84b0552a1375726d9a2fd6213b49e13f49027c3d 
>   src/master/http.cpp d7cd89f0a3446f4c2e65ecd259544149bf92faf8 
>   src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4 
>   src/master/master.cpp 38c553249a11745ca96279c755da8c36bbb3c053 
>   src/master/registrar.cpp 61fdea3a300eabf5037288a6356a04b07c363980 
>   src/slave/http.cpp 1358810cde91fc64d190196e0a7675fa30d8f106 
>   src/slave/isolator.hpp fc13089664e136936ed30ae4a5a11ab55d7719dc 
>   src/slave/slave.hpp b00f97057fd2f9ba72364cb41c543bd24a97b0fa 
>   src/slave/slave.cpp 396293b1749afcbebdf366f4105b46e8ec7749de 
>   src/slave/status_update_manager.cpp f7a0c40f1b1a8cfab828998146b8f446a0569251 
>   src/state/state.hpp 02620b3292849ec7cb85cdf3b1de8bd0796ae597 
> 
> Diff: https://reviews.apache.org/r/16662/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 16662: Refactorings necessary to get Mesos to compile with clang.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16662/#review31276
-----------------------------------------------------------

Ship it!



src/master/registrar.cpp
<https://reviews.apache.org/r/16662/#comment59667>

    Wouldn't the usual wrapping and indent be:
    
    LOG(WARNING) << "Failed to recover registrar: "
                 << (recovery.isFailed() ...
    ?



src/slave/slave.hpp
<https://reviews.apache.org/r/16662/#comment59666>

    Do you still want this in?


- Niklas Nielsen


On Jan. 6, 2014, 7:40 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16662/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2014, 7:40 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 9b258347c79bf54a85cc64fc84ca7a433d8d847f 
>   include/mesos/scheduler.hpp 161cc659bf971a758406d42573a43299327f0b43 
>   src/files/files.cpp 45ef95cd40c4cf2d9235668860b032e96cc81066 
>   src/master/allocator.hpp 85ed2141b4539f383ad9e2527321e066cb5429f6 
>   src/master/contender.cpp 84b0552a1375726d9a2fd6213b49e13f49027c3d 
>   src/master/http.cpp d7cd89f0a3446f4c2e65ecd259544149bf92faf8 
>   src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4 
>   src/master/master.cpp 38c553249a11745ca96279c755da8c36bbb3c053 
>   src/master/registrar.cpp 61fdea3a300eabf5037288a6356a04b07c363980 
>   src/slave/http.cpp 1358810cde91fc64d190196e0a7675fa30d8f106 
>   src/slave/isolator.hpp fc13089664e136936ed30ae4a5a11ab55d7719dc 
>   src/slave/slave.hpp b00f97057fd2f9ba72364cb41c543bd24a97b0fa 
>   src/slave/slave.cpp 396293b1749afcbebdf366f4105b46e8ec7749de 
>   src/slave/status_update_manager.cpp f7a0c40f1b1a8cfab828998146b8f446a0569251 
>   src/state/state.hpp 02620b3292849ec7cb85cdf3b1de8bd0796ae597 
> 
> Diff: https://reviews.apache.org/r/16662/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>