You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2016/12/13 00:52:57 UTC
Review Request 54686: Moved the `IOSwitchboardServer::isRequired` to
`IOSwitchBoard`.
-----------------------------------------------------------
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
Re: Review Request 54686: Moved the `IOSwitchboardServer::isRequired`
to `IOSwitchBoard`.
Posted by Vinod Kone <vi...@gmail.com>.
> On Dec. 13, 2016, 12:54 a.m., Kevin Klues wrote:
> > src/slave/containerizer/mesos/io/switchboard.hpp, lines 81-99
> > <https://reviews.apache.org/r/54686/diff/1/?file=1582362#file1582362line81>
> >
> > 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`.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/54686/#comment229784>
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
>
>