You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Till Toenshoff <to...@me.com> on 2015/06/08 14:59:16 UTC
Re: Review Request 34703: Added stream manipulators for the Time
object.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34703/#review87013
-----------------------------------------------------------
Looks pretty good. I just am unsure about the class naming - the rest is just nits.
3rdparty/libprocess/include/process/time.hpp
<https://reviews.apache.org/r/34703/#comment139262>
Can we give this a different, more intuitive name please?
I would suggest `HTTPDate` instead.
Can we please also add an example so people looking for a particular format will quickly find it?
3rdparty/libprocess/include/process/time.hpp
<https://reviews.apache.org/r/34703/#comment139263>
3rdparty/libprocess/include/process/time.hpp
<https://reviews.apache.org/r/34703/#comment139264>
`snprintf` does not seem to set `errno`, hence `LOG` should be used instead of `PLOG`.
3rdparty/libprocess/include/process/time.hpp
<https://reviews.apache.org/r/34703/#comment139265>
2 chars indent please.
3rdparty/libprocess/include/process/time.hpp
<https://reviews.apache.org/r/34703/#comment139275>
Lets think about a good replacement for that class name to make it intuitive - first thing that came to mind is ```UnixTimestamp```.
Lets also add an example right here in this comment.
3rdparty/libprocess/include/process/time.hpp
<https://reviews.apache.org/r/34703/#comment139276>
2 chars intention please.
- Till Toenshoff
On May 29, 2015, 12:59 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34703/
> -----------------------------------------------------------
>
> (Updated May 29, 2015, 12:59 p.m.)
>
>
> Review request for mesos, Bernd Mathiske, Michael Park, Nikita Vetoshkin, and Till Toenshoff.
>
>
> Bugs: mesos-708
> https://issues.apache.org/jira/browse/mesos-708
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds some manipulator classes which allows formatting Time objects to ostreams.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/time.hpp c5ab2a3cfa83590eb6612152ae365dd67f51cea9
> 3rdparty/libprocess/src/tests/time_tests.cpp be314182c65c05d439b81aa5248a71d93f6f0a0b
>
> Diff: https://reviews.apache.org/r/34703/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 34703: Added stream manipulators for the Time
object.
Posted by Alexander Rojas <al...@mesosphere.io>.
> On June 8, 2015, 2:59 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/include/process/time.hpp, line 150
> > <https://reviews.apache.org/r/34703/diff/3/?file=974298#file974298line150>
> >
> > Lets think about a good replacement for that class name to make it intuitive - first thing that came to mind is ```UnixTimestamp```.
> >
> > Lets also add an example right here in this comment.
Apparently BenH is OK with leaving the RFC as part of the name.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34703/#review87013
-----------------------------------------------------------
On June 8, 2015, 5:14 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34703/
> -----------------------------------------------------------
>
> (Updated June 8, 2015, 5:14 p.m.)
>
>
> Review request for mesos, Bernd Mathiske, Michael Park, Nikita Vetoshkin, and Till Toenshoff.
>
>
> Bugs: mesos-708
> https://issues.apache.org/jira/browse/mesos-708
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds some manipulator classes which allows formatting Time objects to ostreams.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/time.hpp c5ab2a3cfa83590eb6612152ae365dd67f51cea9
> 3rdparty/libprocess/src/tests/time_tests.cpp be314182c65c05d439b81aa5248a71d93f6f0a0b
>
> Diff: https://reviews.apache.org/r/34703/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>