You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Patrick Reilly <pa...@gmail.com> on 2014/08/27 01:10:21 UTC
Review Request 25084: Switched to single variadic 'join' function
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25084/
-----------------------------------------------------------
Review request for mesos, Adam B and Benjamin Hindman.
Bugs: MESOS-1733
https://issues.apache.org/jira/browse/MESOS-1733
Repository: mesos-git
Description
-------
Changed the stout path utility to declare a single, variadic 'join' function instead of several separate declarations of various discrete arities
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a
Diff: https://reviews.apache.org/r/25084/diff/
Testing
-------
Ran "make check" working on creating a new unit test.
Thanks,
Patrick Reilly
Re: Review Request 25084: Switched to single variadic 'join' function
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25084/#review51945
-----------------------------------------------------------
Patch looks great!
Reviews applied: [25084]
All tests passed.
- Mesos ReviewBot
On Aug. 29, 2014, 7:07 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25084/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 7:07 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-1733
> https://issues.apache.org/jira/browse/MESOS-1733
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Changed the stout path utility to declare a single, variadic 'join' function instead of several separate declarations of various discrete arities
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am db9766d
> 3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de
> 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a
> 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25084/diff/
>
>
> Testing
> -------
>
> Ran "make check" created path_tests unit test.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25084: Switched to single variadic 'join' function
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25084/
-----------------------------------------------------------
(Updated Aug. 29, 2014, 7:07 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Flipping ifdef logic, and addressing style issues.
Bugs: MESOS-1733
https://issues.apache.org/jira/browse/MESOS-1733
Repository: mesos-git
Description
-------
Changed the stout path utility to declare a single, variadic 'join' function instead of several separate declarations of various discrete arities
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/Makefile.am db9766d
3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/25084/diff/
Testing
-------
Ran "make check" created path_tests unit test.
Thanks,
Patrick Reilly
Re: Review Request 25084: Switched to single variadic 'join' function
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25084/#review51868
-----------------------------------------------------------
Patch looks great!
Reviews applied: [25084]
All tests passed.
- Mesos ReviewBot
On Aug. 28, 2014, 12:51 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25084/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2014, 12:51 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-1733
> https://issues.apache.org/jira/browse/MESOS-1733
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Changed the stout path utility to declare a single, variadic 'join' function instead of several separate declarations of various discrete arities
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am db9766d
> 3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de
> 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a
> 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25084/diff/
>
>
> Testing
> -------
>
> Ran "make check" created path_tests unit test.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25084: Switched to single variadic 'join' function
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25084/#review51743
-----------------------------------------------------------
Looks pretty good. Just a couple questions about #if logic, testing trailing slashes, and minor style nits.
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
<https://reviews.apache.org/r/25084/#comment90311>
Shouldn't this #if logic be reversed, such that "strings.hpp" is included if < 2011?
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
<https://reviews.apache.org/r/25084/#comment90314>
Style nit: We typically use "char* foo" over "char *foo"
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
<https://reviews.apache.org/r/25084/#comment90322>
We weren't trimming trailing slashes before. Would this be a difference in behavior between the C++11 version and the old version?
Did you run the unit tests both with & without C++11 enabled?
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
<https://reviews.apache.org/r/25084/#comment90315>
What happens here if part==NULL? then end = NULL and we try to decrement it? Probably should assert/early-exit if (part==NULL)
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
<https://reviews.apache.org/r/25084/#comment90318>
Style nit: Either end this comment with punctuation ('.') or remove it.
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp
<https://reviews.apache.org/r/25084/#comment90323>
Please add Apache license block.
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp
<https://reviews.apache.org/r/25084/#comment90325>
Two of these look identical. Am I blind, or did you mean for one to test chars with join('a', 'b', 'c')?
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp
<https://reviews.apache.org/r/25084/#comment90326>
Do these trailing slash tests pass without your new C++11 changes? Does the old code need to be updated?
- Adam B
On Aug. 27, 2014, 5:51 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25084/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2014, 5:51 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-1733
> https://issues.apache.org/jira/browse/MESOS-1733
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Changed the stout path utility to declare a single, variadic 'join' function instead of several separate declarations of various discrete arities
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am db9766d
> 3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de
> 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a
> 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25084/diff/
>
>
> Testing
> -------
>
> Ran "make check" created path_tests unit test.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25084: Switched to single variadic 'join' function
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25084/
-----------------------------------------------------------
(Updated Aug. 28, 2014, 12:51 a.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Added guard for C++11 parts with ifdef.
Bugs: MESOS-1733
https://issues.apache.org/jira/browse/MESOS-1733
Repository: mesos-git
Description
-------
Changed the stout path utility to declare a single, variadic 'join' function instead of several separate declarations of various discrete arities
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/Makefile.am db9766d
3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/25084/diff/
Testing
-------
Ran "make check" created path_tests unit test.
Thanks,
Patrick Reilly
Re: Review Request 25084: Switched to single variadic 'join' function
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25084/
-----------------------------------------------------------
(Updated Aug. 27, 2014, 12:39 a.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Bugs: MESOS-1733
https://issues.apache.org/jira/browse/MESOS-1733
Repository: mesos-git
Description
-------
Changed the stout path utility to declare a single, variadic 'join' function instead of several separate declarations of various discrete arities
Diffs
-----
3rdparty/libprocess/3rdparty/Makefile.am db9766d
3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/25084/diff/
Testing (updated)
-------
Ran "make check" created path_tests unit test.
Thanks,
Patrick Reilly
Re: Review Request 25084: Switched to single variadic 'join' function
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25084/
-----------------------------------------------------------
(Updated Aug. 27, 2014, 12:38 a.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Added path_tests to Makefile in libprocess/3rdparty. Test now runs during make check. It also passes.
Bugs: MESOS-1733
https://issues.apache.org/jira/browse/MESOS-1733
Repository: mesos-git
Description
-------
Changed the stout path utility to declare a single, variadic 'join' function instead of several separate declarations of various discrete arities
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/Makefile.am db9766d
3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/25084/diff/
Testing
-------
Ran "make check" working on creating a new unit test.
Thanks,
Patrick Reilly
Re: Review Request 25084: Switched to single variadic 'join' function
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25084/
-----------------------------------------------------------
(Updated Aug. 26, 2014, 11:56 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Added unit test for stout/path, but it doesn't seem to be running. Also closed some issues to do with forming absolute paths.
Bugs: MESOS-1733
https://issues.apache.org/jira/browse/MESOS-1733
Repository: mesos-git
Description
-------
Changed the stout path utility to declare a single, variadic 'join' function instead of several separate declarations of various discrete arities
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/25084/diff/
Testing
-------
Ran "make check" working on creating a new unit test.
Thanks,
Patrick Reilly