Re: Review Request 54686: Moved the `IOSwitchboardServer::isRequired` to `IOSwitchBoard`.

2016-12-12 Thread Vinod Kone


> On Dec. 13, 2016, 12:54 a.m., Kevin Klues wrote:
> > src/slave/containerizer/mesos/io/switchboard.hpp, lines 81-99
> > 
> >
> > Is there a reason not to put the implementation of this in the .cpp 
> > file? Seems weird to have all other functions defined in the cpp file, but 
> > define this one in the header.

good point. i'll do this in a subsequent review.


- Vinod


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


On Dec. 13, 2016, 12:52 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54686/
> ---
> 
> (Updated Dec. 13, 2016, 12:52 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed it to `IOSwitchboard::requiresServer` after the move.
> 
> This is mainly done for supporting Windows builds because
> `IOSwitchboardServer` is ifdef'ed out for Windows but we need access
> to the static method.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> 9f4ce99d09d50b681e01b5d28fb5fc79e1ea85fc 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> f900924dd55c42966deb14c65fca380bebc86e2f 
> 
> Diff: https://reviews.apache.org/r/54686/diff/
> 
> 
> Testing
> ---
> 
> make -j20 check GTEST_FILTER="*AgentAPI*:*Switchboard*" MESOS_VERBOSE=1
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 54686: Moved the `IOSwitchboardServer::isRequired` to `IOSwitchBoard`.

2016-12-12 Thread Kevin Klues

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


Fix it, then Ship it!





src/slave/containerizer/mesos/io/switchboard.hpp (lines 81 - 99)


Is there a reason not to put the implementation of this in the .cpp file? 
Seems weird to have all other functions defined in the cpp file, but define 
this one in the header.


- Kevin Klues


On Dec. 13, 2016, 12:52 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54686/
> ---
> 
> (Updated Dec. 13, 2016, 12:52 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed it to `IOSwitchboard::requiresServer` after the move.
> 
> This is mainly done for supporting Windows builds because
> `IOSwitchboardServer` is ifdef'ed out for Windows but we need access
> to the static method.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> 9f4ce99d09d50b681e01b5d28fb5fc79e1ea85fc 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> f900924dd55c42966deb14c65fca380bebc86e2f 
> 
> Diff: https://reviews.apache.org/r/54686/diff/
> 
> 
> Testing
> ---
> 
> make -j20 check GTEST_FILTER="*AgentAPI*:*Switchboard*" MESOS_VERBOSE=1
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Review Request 54686: Moved the `IOSwitchboardServer::isRequired` to `IOSwitchBoard`.

2016-12-12 Thread Vinod Kone

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

Review request for mesos, Alex Clemmer and Kevin Klues.


Repository: mesos


Description
---

Renamed it to `IOSwitchboard::requiresServer` after the move.

This is mainly done for supporting Windows builds because
`IOSwitchboardServer` is ifdef'ed out for Windows but we need access
to the static method.


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.hpp 
9f4ce99d09d50b681e01b5d28fb5fc79e1ea85fc 
  src/slave/containerizer/mesos/io/switchboard.cpp 
f900924dd55c42966deb14c65fca380bebc86e2f 

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


Testing
---

make -j20 check GTEST_FILTER="*AgentAPI*:*Switchboard*" MESOS_VERBOSE=1


Thanks,

Vinod Kone