You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jeff Coffler <je...@taltos.com> on 2017/10/11 23:31:06 UTC
Re: Review Request 60622: Added new stout functions for path
normalizaiton and URI conversion.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60622/
-----------------------------------------------------------
(Updated Oct. 11, 2017, 11:31 p.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Summary (updated)
-----------------
Added new stout functions for path normalizaiton and URI conversion.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description (updated)
-------
Added new stout function: path::normalize (normalize a filename),
along with uri::from_path (generate URI from file path).
Diffs (updated)
-----
3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5
3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751
3rdparty/stout/include/stout/path.hpp 6ee3a44cd6a878fe383aa68df40b82857b93d0b4
3rdparty/stout/include/stout/uri.hpp PRE-CREATION
3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd
3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d
3rdparty/stout/tests/uri_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/60622/diff/3/
Changes: https://reviews.apache.org/r/60622/diff/2-3/
Testing
-------
See upstream
Thanks,
Jeff Coffler
Re: Review Request 60622: Added new stout functions for path and URI
conversions.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60622/
-----------------------------------------------------------
(Updated Oct. 19, 2017, 9:33 p.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Added new stout function: path::from_uri, which converts to a filepath
from a URI.
Also added uri::from_path, which generates a URI from a file path.
Diffs (updated)
-----
3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5
3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751
3rdparty/stout/include/stout/path.hpp 6ee3a44cd6a878fe383aa68df40b82857b93d0b4
3rdparty/stout/include/stout/uri.hpp PRE-CREATION
3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd
3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d
3rdparty/stout/tests/uri_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/60622/diff/6/
Changes: https://reviews.apache.org/r/60622/diff/5-6/
Testing
-------
See upstream
Thanks,
Jeff Coffler
Re: Review Request 60622: Added new stout functions for path and URI
conversions.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60622/#review188727
-----------------------------------------------------------
Ship it!
Ship It!
- Andrew Schwartzmeyer
On Oct. 19, 2017, 11:16 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60622/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 11:16 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added new stout function: path::from_uri, which converts to a filepath
> from a URI.
>
> Also added uri::from_path, which generates a URI from a file path.
>
>
> Diffs
> -----
>
> 3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5
> 3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751
> 3rdparty/stout/include/stout/path.hpp 6ee3a44cd6a878fe383aa68df40b82857b93d0b4
> 3rdparty/stout/include/stout/uri.hpp PRE-CREATION
> 3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd
> 3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d
> 3rdparty/stout/tests/uri_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/60622/diff/5/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60622: Added new stout functions for path and URI
conversions.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60622/
-----------------------------------------------------------
(Updated Oct. 19, 2017, 6:16 p.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Added new stout function: path::from_uri, which converts to a filepath
from a URI.
Also added uri::from_path, which generates a URI from a file path.
Diffs (updated)
-----
3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5
3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751
3rdparty/stout/include/stout/path.hpp 6ee3a44cd6a878fe383aa68df40b82857b93d0b4
3rdparty/stout/include/stout/uri.hpp PRE-CREATION
3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd
3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d
3rdparty/stout/tests/uri_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/60622/diff/5/
Changes: https://reviews.apache.org/r/60622/diff/4-5/
Testing
-------
See upstream
Thanks,
Jeff Coffler
Re: Review Request 60622: Added new stout functions for path and URI
conversions.
Posted by Jeff Coffler <je...@taltos.com>.
> On Oct. 17, 2017, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 45-52 (patched)
> > <https://reviews.apache.org/r/60622/diff/4/?file=1858861#file1858861line45>
> >
> > The `startsWith` is unecessary. This can be replaced with one line:
> >
> > ```
> > std::string path = strings::remove(path, "file://", strings::PREFIX);`
> > ```
> >
> > Since `strings::remove` is idempotent.
Changed.
> On Oct. 17, 2017, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 47 (patched)
> > <https://reviews.apache.org/r/60622/diff/4/?file=1858861#file1858861line47>
> >
> > nit: comments must end with `.`
Ok.
> On Oct. 17, 2017, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 48 (patched)
> > <https://reviews.apache.org/r/60622/diff/4/?file=1858861#file1858861line48>
> >
> > nit: whitespace
Ok.
> On Oct. 17, 2017, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 49-50 (patched)
> > <https://reviews.apache.org/r/60622/diff/4/?file=1858861#file1858861line49>
> >
> > nit: open brace should be on line with `if`
Refactored out due to prior change.
> On Oct. 17, 2017, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/uri.hpp
> > Lines 24 (patched)
> > <https://reviews.apache.org/r/60622/diff/4/?file=1858862#file1858862line24>
> >
> > nit: comment must end with a `.`, and this should perhaps mention why we replace `` with `/` on Windows.
Ok.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60622/#review188414
-----------------------------------------------------------
On Oct. 17, 2017, 1:17 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60622/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:17 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added new stout function: path::from_uri, which converts to a filepath
> from a URI.
>
> Also added uri::from_path, which generates a URI from a file path.
>
>
> Diffs
> -----
>
> 3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5
> 3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751
> 3rdparty/stout/include/stout/path.hpp 6ee3a44cd6a878fe383aa68df40b82857b93d0b4
> 3rdparty/stout/include/stout/uri.hpp PRE-CREATION
> 3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd
> 3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d
> 3rdparty/stout/tests/uri_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/60622/diff/4/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60622: Added new stout functions for path and URI
conversions.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60622/#review188414
-----------------------------------------------------------
3rdparty/stout/include/stout/path.hpp
Lines 45-52 (patched)
<https://reviews.apache.org/r/60622/#comment265415>
The `startsWith` is unecessary. This can be replaced with one line:
```
std::string path = strings::remove(path, "file://", strings::PREFIX);`
```
Since `strings::remove` is idempotent.
3rdparty/stout/include/stout/path.hpp
Lines 47 (patched)
<https://reviews.apache.org/r/60622/#comment265414>
nit: comments must end with `.`
3rdparty/stout/include/stout/path.hpp
Lines 48 (patched)
<https://reviews.apache.org/r/60622/#comment265411>
nit: whitespace
3rdparty/stout/include/stout/path.hpp
Lines 49-50 (patched)
<https://reviews.apache.org/r/60622/#comment265413>
nit: open brace should be on line with `if`
3rdparty/stout/include/stout/uri.hpp
Lines 24 (patched)
<https://reviews.apache.org/r/60622/#comment265416>
nit: comment must end with a `.`, and this should perhaps mention why we replace `` with `/` on Windows.
- Andrew Schwartzmeyer
On Oct. 16, 2017, 6:17 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60622/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:17 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added new stout function: path::from_uri, which converts to a filepath
> from a URI.
>
> Also added uri::from_path, which generates a URI from a file path.
>
>
> Diffs
> -----
>
> 3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5
> 3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751
> 3rdparty/stout/include/stout/path.hpp 6ee3a44cd6a878fe383aa68df40b82857b93d0b4
> 3rdparty/stout/include/stout/uri.hpp PRE-CREATION
> 3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd
> 3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d
> 3rdparty/stout/tests/uri_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/60622/diff/4/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60622: Added new stout functions for path and URI
conversions.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60622/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 1:17 a.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Summary (updated)
-----------------
Added new stout functions for path and URI conversions.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description (updated)
-------
Added new stout function: path::from_uri, which converts to a filepath
from a URI.
Also added uri::from_path, which generates a URI from a file path.
Diffs (updated)
-----
3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5
3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751
3rdparty/stout/include/stout/path.hpp 6ee3a44cd6a878fe383aa68df40b82857b93d0b4
3rdparty/stout/include/stout/uri.hpp PRE-CREATION
3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd
3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d
3rdparty/stout/tests/uri_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/60622/diff/4/
Changes: https://reviews.apache.org/r/60622/diff/3-4/
Testing
-------
See upstream
Thanks,
Jeff Coffler
Re: Review Request 60622: Added new stout functions for path
normalizaiton and URI conversion.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60622/#review188011
-----------------------------------------------------------
3rdparty/stout/include/stout/path.hpp
Lines 28-46 (patched)
<https://reviews.apache.org/r/60622/#comment265081>
This should be replaced with `path::from_uri` which validates the `file://` portion, and fixes the slashes on Windows, as mentioned in the other review.
3rdparty/stout/include/stout/uri.hpp
Lines 24-27 (patched)
<https://reviews.apache.org/r/60622/#comment265090>
After looking at the other reviews, the reason for using this publicly appears dubious. It's bandaging the incorrect construction of URLs that used `path::join` to join their components, a pattern that should be avoided in cross-platform code.
- Andrew Schwartzmeyer
On Oct. 11, 2017, 4:31 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60622/
> -----------------------------------------------------------
>
> (Updated Oct. 11, 2017, 4:31 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added new stout function: path::normalize (normalize a filename),
> along with uri::from_path (generate URI from file path).
>
>
> Diffs
> -----
>
> 3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5
> 3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751
> 3rdparty/stout/include/stout/path.hpp 6ee3a44cd6a878fe383aa68df40b82857b93d0b4
> 3rdparty/stout/include/stout/uri.hpp PRE-CREATION
> 3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd
> 3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d
> 3rdparty/stout/tests/uri_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/60622/diff/3/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>