You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2016/03/02 11:34:45 UTC
Review Request 44261: Introduced a RAII helper for
process::metrics::Timer.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44261/
-----------------------------------------------------------
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Bugs: MESOS-4721
https://issues.apache.org/jira/browse/MESOS-4721
Repository: mesos
Description
-------
Introduced a RAII helper for process::metrics::Timer.
Diffs
-----
3rdparty/libprocess/include/process/metrics/timer.hpp 0a9c0227c457c6c81a59f65f901a5464ee00983d
Diff: https://reviews.apache.org/r/44261/diff/
Testing
-------
Thanks,
Benjamin Bannier
Re: Review Request 44261: Introduced a RAII helper for
process::metrics::Timer.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44261/
-----------------------------------------------------------
(Updated March 3, 2016, 5:17 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Addressed comments form Alex.
Bugs: MESOS-4721
https://issues.apache.org/jira/browse/MESOS-4721
Repository: mesos
Description (updated)
-------
Introduced a RAII helper for process::metrics::Timer.
Diffs (updated)
-----
3rdparty/libprocess/include/process/metrics/timer.hpp 0a9c0227c457c6c81a59f65f901a5464ee00983d
Diff: https://reviews.apache.org/r/44261/diff/
Testing
-------
`make check` succeeds under OS X (but this patch just adds a header). The follow-up patch using this header does not need further changes here though.
Thanks,
Benjamin Bannier
Re: Review Request 44261: Introduced a RAII helper for
process::metrics::Timer.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44261/
-----------------------------------------------------------
(Updated March 3, 2016, 2:11 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Style: bracket wrapping.
Bugs: MESOS-4721
https://issues.apache.org/jira/browse/MESOS-4721
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/include/process/metrics/timer.hpp 0a9c0227c457c6c81a59f65f901a5464ee00983d
Diff: https://reviews.apache.org/r/44261/diff/
Testing
-------
`make check` succeeds under OS X (but this patch just adds a header). The follow-up patch using this header does not need further changes here though.
Thanks,
Benjamin Bannier
Re: Review Request 44261: Introduced a RAII helper for
process::metrics::Timer.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44261/
-----------------------------------------------------------
(Updated March 2, 2016, 4:43 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Minor style fixes.
Bugs: MESOS-4721
https://issues.apache.org/jira/browse/MESOS-4721
Repository: mesos
Description (updated)
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/include/process/metrics/timer.hpp 0a9c0227c457c6c81a59f65f901a5464ee00983d
Diff: https://reviews.apache.org/r/44261/diff/
Testing (updated)
-------
`make check` succeeds under OS X (but this patch just adds a header). The follow-up patch using this header does not need further changes here though.
Thanks,
Benjamin Bannier
Re: Review Request 44261: Introduced a RAII helper for
process::metrics::Timer.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On March 2, 2016, 2:52 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/include/process/metrics/timer.hpp, line 147
> > <https://reviews.apache.org/r/44261/diff/1/?file=1276482#file1276482line147>
> >
> > I don't think we put non-empty function body on the same line.
In fact we do (see e.g. Mesos style guide examples on lambdas), and is also consistent with Google style guide (opening bracket on same line, closing on new line or same line as opening bracket).
Broke this into multiple lines now nevertheless.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44261/#review121635
-----------------------------------------------------------
On March 2, 2016, 4:43 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44261/
> -----------------------------------------------------------
>
> (Updated March 2, 2016, 4:43 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4721
> https://issues.apache.org/jira/browse/MESOS-4721
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/metrics/timer.hpp 0a9c0227c457c6c81a59f65f901a5464ee00983d
>
> Diff: https://reviews.apache.org/r/44261/diff/
>
>
> Testing
> -------
>
> `make check` succeeds under OS X (but this patch just adds a header). The follow-up patch using this header does not need further changes here though.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 44261: Introduced a RAII helper for
process::metrics::Timer.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44261/#review121635
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/libprocess/include/process/metrics/timer.hpp (line 137)
<https://reviews.apache.org/r/44261/#comment183382>
I think you can omit the `process::metrics::` prefix here and below.
For consistency, please s/`timer_`/`_timer`.
3rdparty/libprocess/include/process/metrics/timer.hpp (line 147)
<https://reviews.apache.org/r/44261/#comment183383>
I don't think we put non-empty function body on the same line.
- Alexander Rukletsov
On March 2, 2016, 10:34 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44261/
> -----------------------------------------------------------
>
> (Updated March 2, 2016, 10:34 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4721
> https://issues.apache.org/jira/browse/MESOS-4721
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Introduced a RAII helper for process::metrics::Timer.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/metrics/timer.hpp 0a9c0227c457c6c81a59f65f901a5464ee00983d
>
> Diff: https://reviews.apache.org/r/44261/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benjamin Bannier
>
>