mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 42860: Add paths::sameParent to find out the same root of a path list.
Date Wed, 27 Jan 2016 21:06:27 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42860/#review116606
-----------------------------------------------------------




3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 68)
<https://reviews.apache.org/r/42860/#comment177675>

    I am extremely confused about the expected semantics of this function (e.g. `EXPECT_SOME_EQ("/",
path::sameParent({"/usr", "/", "/tmp"}));`?). I feel developing an intuition would be easier
would this function take just two args (but am unsure); also, the name seems to suggest this
returning some `bool` type which it doesn't.
    
    Looking at the implementation isn't too helpful either due to the complicated control
flow. It should in principle be possible to simplify this by separating extracting path components
and their comparisons, but am unsure if this is something we need for the intended use case
from the follow-up patch.



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (lines 70 - 71)
<https://reviews.apache.org/r/42860/#comment177673>

    These will dereference `paths.end()` if `paths` is empty (which in turn might do anything).


- Benjamin Bannier


On Jan. 27, 2016, 7:36 p.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42860/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2016, 7:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4521
>     https://issues.apache.org/jira/browse/MESOS-4521
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add paths::sameParent to find out the same root of a path list.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp ef538045a8b7a1e3d8962c869317d86a85e0259f

>   3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 6dff5e76e0e15098c5a262adc50bfcb65f933697

> 
> Diff: https://reviews.apache.org/r/42860/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> haosdent huang
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message