mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinod Kone" <vinodk...@gmail.com>
Subject Re: Review Request: Fixes operations on Ranges
Date Tue, 31 Jul 2012 17:05:37 GMT


> On July 31, 2012, 6:17 a.m., Bill Farner wrote:
> > src/common/values.cpp, line 287
> > <https://reviews.apache.org/r/6224/diff/2/?file=131052#file131052line287>
> >
> >     mismatch in indent/wrap style - some places the operator is on the first line,
others it's on the continued line

fixed


> On July 31, 2012, 6:17 a.m., Bill Farner wrote:
> > src/common/values.cpp, line 203
> > <https://reviews.apache.org/r/6224/diff/2/?file=131052#file131052line203>
> >
> >     Some inner parens would be nice to prevent second-guessing the order of operations.
> >     
> >     Also, ASCII art would be fantastic - i had to stare at this for a bit to be
convinced that it works.
> >     Feel free to steal my work below.
> >     
> >     //                    b         e
> >     // current        |- - - -| 
> >     // range    |- - - -|
> >     //              b         e 
> >     // current  |- - - -| 
> >     // range          |- - - -| 
> >     //                    b          e
> 
> Bill Farner wrote:
>     Err, well it looked nice in the non fixed-width font for the comment editor, you'd
have to adjust the spacing for the b/e markers.

added ascii art similar to other functions in this file


> On July 31, 2012, 6:17 a.m., Bill Farner wrote:
> > src/common/values.cpp, line 272
> > <https://reviews.apache.org/r/6224/diff/2/?file=131052#file131052line272>
> >
> >     this block is repeated 4 times, a helper function would be nice.

overloaded coalesce() to make this simpler


- Vinod


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


On July 31, 2012, 5:42 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6224/
> -----------------------------------------------------------
> 
> (Updated July 31, 2012, 5:42 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and John Sirois.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> This addresses bug MESOS-247.
>     https://issues.apache.org/jira/browse/MESOS-247
> 
> 
> Diffs
> -----
> 
>   src/common/values.cpp 0e9dd4f 
>   src/tests/resources_tests.cpp 3e144ed 
> 
> Diff: https://reviews.apache.org/r/6224/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Please let me know any other tests you can think of. Lets fix ranges foreva.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


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