Review Request 25084: Switched to single variadic 'join' function

2014-08-26 Thread Patrick Reilly 

---
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

2014-08-26 Thread Patrick Reilly 

---
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

Re: Review Request 25084: Switched to single variadic 'join' function

2014-08-26 Thread Patrick Reilly 

---
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

2014-08-26 Thread Patrick Reilly 

---
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

2014-08-27 Thread Patrick Reilly 

---
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

2014-08-27 Thread Adam B 

---
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


Shouldn't this #if logic be reversed, such that "strings.hpp" is included 
if < 2011?



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp


Style nit: We typically use "char* foo" over "char *foo"



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp


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


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


Style nit: Either end this comment with punctuation ('.') or remove it.



3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp


Please add Apache license block.



3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp


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


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

2014-08-28 Thread Mesos ReviewBot 

---
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

2014-08-29 Thread Patrick Reilly 

---
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

2014-08-30 Thread Mesos ReviewBot 

---
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
> 
>