You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2016/01/15 20:34:18 UTC
Review Request 42358: Logger Module: Refactored Sandbox logger
initialization.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42358/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
Bugs: MESOS-4136
https://issues.apache.org/jira/browse/MESOS-4136
Repository: mesos
Description
-------
In retrospect, the `if (process != NULL)` initialization check in each sandbox logger function is unnecessary for such a simple module.
Diffs
-----
src/slave/container_loggers/sandbox.hpp 17bb1d9791d46c834e7d074fc72c95f093cad990
src/slave/container_loggers/sandbox.cpp 1b954d64ba97f650b1024eed383ca46093ca1b6b
Diff: https://reviews.apache.org/r/42358/diff/
Testing
-------
make check
Thanks,
Joseph Wu
Re: Review Request 42358: Logger Module: Refactored Sandbox logger
initialization.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42358/
-----------------------------------------------------------
(Updated Jan. 22, 2016, 11:55 p.m.)
Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
Changes
-------
Moved `Process` init into the constructor's initializer list.
Bugs: MESOS-4136
https://issues.apache.org/jira/browse/MESOS-4136
Repository: mesos
Description
-------
In retrospect, the `if (process != NULL)` initialization check in each sandbox logger function is unnecessary for such a simple module.
Diffs (updated)
-----
src/slave/container_loggers/sandbox.hpp 17bb1d9791d46c834e7d074fc72c95f093cad990
src/slave/container_loggers/sandbox.cpp 1b954d64ba97f650b1024eed383ca46093ca1b6b
Diff: https://reviews.apache.org/r/42358/diff/
Testing
-------
make check
Thanks,
Joseph Wu
Re: Review Request 42358: Logger Module: Refactored Sandbox logger
initialization.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42358/#review115097
-----------------------------------------------------------
Ship it!
I'll fix these up and commit, thanks!
src/slave/container_loggers/sandbox.cpp (line 77)
<https://reviews.apache.org/r/42358/#comment175960>
We should initialize `process` using an initializer list. This is the common pattern in the code base too.
src/slave/container_loggers/sandbox.cpp (line 84)
<https://reviews.apache.org/r/42358/#comment175961>
Don't need this if this is initialized in the constructor initializer list (and this is the common pattern in the code base).
- Benjamin Hindman
On Jan. 15, 2016, 7:34 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42358/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2016, 7:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
>
>
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In retrospect, the `if (process != NULL)` initialization check in each sandbox logger function is unnecessary for such a simple module.
>
>
> Diffs
> -----
>
> src/slave/container_loggers/sandbox.hpp 17bb1d9791d46c834e7d074fc72c95f093cad990
> src/slave/container_loggers/sandbox.cpp 1b954d64ba97f650b1024eed383ca46093ca1b6b
>
> Diff: https://reviews.apache.org/r/42358/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>