mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam B" <a...@mesosphere.io>
Subject Re: Review Request 35179: MESOS-1733 Variadic Path Join
Date Fri, 12 Jun 2015 11:15:12 GMT

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


Looks like Cody already made path::join() variadic in commit b08fccf8f5ea325b8c38055b5f2c03509744dd9b
"Switched path::join() to be variadic". How is your patch an improvement? What problem is
it solving?

Please remove the "Might have some C++ style issues..." line in your description, since the
apply-review script uses the description to create the commit message. You can add any comments
to reviewers in the Testing section, since that is ignored by apply-review.

Your Testing Done section says that you "added some additional tests", but I don't see them
in this patch. Looks like they were in a previous revision, but removed. Please add them back
or update the Testing section.

ReviewBot says the patch doesn't apply cleanly, so please rebase.

- Adam B


On June 12, 2015, 3:32 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35179/
> -----------------------------------------------------------
> 
> (Updated June 12, 2015, 3:32 a.m.)
> 
> 
> Review request for mesos, Adam B and Cody Maloney.
> 
> 
> Bugs: MESOS-1733
>     https://issues.apache.org/jira/browse/MESOS-1733
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change takes an un-complicated/naive route ( no trimming of values etc ) at making
path::join(...) variadic mainly in order to preserve the earlier over-loaded join functionality.
> 
> Might have some C++ style issues owing to this being my first commit here.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp d4df6502d1297ea3ad8e2a1e3bb16ea9d7c7913c

> 
> Diff: https://reviews.apache.org/r/35179/diff/
> 
> 
> Testing
> -------
> 
> make check + added some additional tests.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


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