mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Toenshoff" <toensh...@me.com>
Subject Re: Review Request 35998: Added doxygen styled comments to Path::basename and Path::dirname.
Date Thu, 09 Jul 2015 11:50:54 GMT


> On July 7, 2015, 9:50 a.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp, line 39
> > <https://reviews.apache.org/r/35998/diff/1/?file=994383#file994383line39>
> >
> >     Do you mean std::basename()?

Nope, the free C function `basename()` is part of the POSIX pattern matching implementations
and as such not part of the `std` namespace (not even wrapped). We commonly mark such function
using the explicit, global namespace `::` in our C++ codebase.


> On July 7, 2015, 9:50 a.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp, line 24
> > <https://reviews.apache.org/r/35998/diff/1/?file=994383#file994383line24>
> >
> >     I know that in many other places we have written "Basic abstraction", but this
really adds no meaning. So let's stop that! 
> >     
> >     And we don't deal with just about any file system here (e.g. FAT32), do we?
> >     
> >     Suggestion:
> >     
> >     "Represents UNIX file systems paths and offers common path manipulations."
> >     
> >     Or something like that :-)

Yep. Good point, let update that as well.


> On July 7, 2015, 9:50 a.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp, line 48
> > <https://reviews.apache.org/r/35998/diff/1/?file=994383#file994383line48>
> >
> >     It would be good to explain why we are opting this way. How is this useful?
OK, maybe nobody knows and we don't want to touch the code? There are  various other inexplicable
outcomes here.

While these may seem inexplicable, they are defined for the standard implementation of this
function. I guess what you would like to see is something more along the lines of a `dirname()`
and `filename()` to produce results that are more straightforward.

I think adding those as an additional implementation is valuable and Cody expressed that as
well in the past.


- Till


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


On June 29, 2015, 11:43 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35998/
> -----------------------------------------------------------
> 
> (Updated June 29, 2015, 11:43 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> see summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp a4afdad0b5f053186ace4d6a37b41cd02e7d415b

> 
> Diff: https://reviews.apache.org/r/35998/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


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