You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2016/01/07 21:00:56 UTC
Review Request 41429: Cleaned up the CfsFilter and clarified its
logging message.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41429/
-----------------------------------------------------------
Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and Timothy Chen.
Repository: mesos
Description
-------
Cleaned up the CfsFilter and clarified its logging message. Many thanks to @bmahler for noticing these issues!
Diffs
-----
src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
Diff: https://reviews.apache.org/r/41429/diff/
Testing
-------
`sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run correctly on Ubuntu, while the appropriate error message is displayed on Debian and the "CFS_" tests are filtered out.
Thanks,
Greg Mann
Re: Review Request 41429: Cleaned up the CfsFilter and clarified its
logging message.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41429/#review113535
-----------------------------------------------------------
Ship it!
Ship It!
- Timothy Chen
On Jan. 8, 2016, 4:24 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41429/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2016, 4:24 p.m.)
>
>
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Cleaned up the CfsFilter and clarified its logging message. Many thanks to @bmahler for noticing these issues!
>
>
> Diffs
> -----
>
> src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
>
> Diff: https://reviews.apache.org/r/41429/diff/
>
>
> Testing
> -------
>
> `sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run correctly on Ubuntu, while the appropriate error message is displayed on Debian and the "CFS_" tests are filtered out.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 41429: Cleaned up the CfsFilter and clarified its
logging message.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41429/#review113510
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41429]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 8, 2016, 4:24 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41429/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2016, 4:24 p.m.)
>
>
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Cleaned up the CfsFilter and clarified its logging message. Many thanks to @bmahler for noticing these issues!
>
>
> Diffs
> -----
>
> src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
>
> Diff: https://reviews.apache.org/r/41429/diff/
>
>
> Testing
> -------
>
> `sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run correctly on Ubuntu, while the appropriate error message is displayed on Debian and the "CFS_" tests are filtered out.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 41429: Cleaned up the CfsFilter and clarified its
logging message.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41429/
-----------------------------------------------------------
(Updated Jan. 8, 2016, 4:24 p.m.)
Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and Timothy Chen.
Changes
-------
Added relevant stout includes and capitalization.
Repository: mesos
Description
-------
Cleaned up the CfsFilter and clarified its logging message. Many thanks to @bmahler for noticing these issues!
Diffs (updated)
-----
src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
Diff: https://reviews.apache.org/r/41429/diff/
Testing
-------
`sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run correctly on Ubuntu, while the appropriate error message is displayed on Debian and the "CFS_" tests are filtered out.
Thanks,
Greg Mann
Re: Review Request 41429: Cleaned up the CfsFilter and clarified its
logging message.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41429/#review113444
-----------------------------------------------------------
Ship it!
Ship It!
src/tests/environment.cpp (line 156)
<https://reviews.apache.org/r/41429/#comment174113>
s/linux/Linux, twice
- Bernd Mathiske
On Jan. 7, 2016, noon, Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41429/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, noon)
>
>
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Cleaned up the CfsFilter and clarified its logging message. Many thanks to @bmahler for noticing these issues!
>
>
> Diffs
> -----
>
> src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
>
> Diff: https://reviews.apache.org/r/41429/diff/
>
>
> Testing
> -------
>
> `sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run correctly on Ubuntu, while the appropriate error message is displayed on Debian and the "CFS_" tests are filtered out.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 41429: Cleaned up the CfsFilter and clarified its
logging message.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41429/#review113335
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41429]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 7, 2016, 8 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41429/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 8 p.m.)
>
>
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Cleaned up the CfsFilter and clarified its logging message. Many thanks to @bmahler for noticing these issues!
>
>
> Diffs
> -----
>
> src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
>
> Diff: https://reviews.apache.org/r/41429/diff/
>
>
> Testing
> -------
>
> `sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run correctly on Ubuntu, while the appropriate error message is displayed on Debian and the "CFS_" tests are filtered out.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 41429: Cleaned up the CfsFilter and clarified its
logging message.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41429/#review113462
-----------------------------------------------------------
Ship it!
src/tests/environment.cpp (line 129)
<https://reviews.apache.org/r/41429/#comment174184>
Include "stout/result.hpp" please.
src/tests/environment.cpp (line 132)
<https://reviews.apache.org/r/41429/#comment174183>
Include "stout/path.hpp" please.
src/tests/environment.cpp (line 135)
<https://reviews.apache.org/r/41429/#comment174185>
Include "stout/none.hpp" please.
src/tests/environment.cpp (line 166)
<https://reviews.apache.org/r/41429/#comment174182>
Include "stout/option.hpp" please.
- Till Toenshoff
On Jan. 7, 2016, 8 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41429/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 8 p.m.)
>
>
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Cleaned up the CfsFilter and clarified its logging message. Many thanks to @bmahler for noticing these issues!
>
>
> Diffs
> -----
>
> src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
>
> Diff: https://reviews.apache.org/r/41429/diff/
>
>
> Testing
> -------
>
> `sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run correctly on Ubuntu, while the appropriate error message is displayed on Debian and the "CFS_" tests are filtered out.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 41429: Cleaned up the CfsFilter and clarified its
logging message.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41429/#review113339
-----------------------------------------------------------
Ship it!
LGTM! It should be more appropriate to use `Option<Error>` instead of a `bool` for `cfsError`.
src/tests/environment.cpp (line 132)
<https://reviews.apache.org/r/41429/#comment173875>
Probably we may want to add header:
`#include <stout/os/exists.hpp>`
- Gilbert Song
On Jan. 7, 2016, noon, Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41429/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, noon)
>
>
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Cleaned up the CfsFilter and clarified its logging message. Many thanks to @bmahler for noticing these issues!
>
>
> Diffs
> -----
>
> src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
>
> Diff: https://reviews.apache.org/r/41429/diff/
>
>
> Testing
> -------
>
> `sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run correctly on Ubuntu, while the appropriate error message is displayed on Debian and the "CFS_" tests are filtered out.
>
>
> Thanks,
>
> Greg Mann
>
>