> On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/common/values.cpp, line 177 > > <https://reviews.apache.org/r/4563/diff/1/?file=97495#file97495line177> > > > > Why not just use DebugString from protobuf and wrap it in an operator > > << in type_utils.hpp?
done > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/common/values.cpp, line 180 > > <https://reviews.apache.org/r/4563/diff/1/?file=97495#file97495line180> > > > > Spaces between '='. n/a > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/common/values.cpp, line 181 > > <https://reviews.apache.org/r/4563/diff/1/?file=97495#file97495line181> > > > > Previous line. n/a > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/common/values.cpp, line 192 > > <https://reviews.apache.org/r/4563/diff/1/?file=97495#file97495line192> > > > > s/temp/range done > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/common/values.cpp, lines 247-248 > > <https://reviews.apache.org/r/4563/diff/1/?file=97495#file97495line247> > > > > Might as well just use range.begin/end below since you're doing that > > above. done > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/common/values.cpp, line 213 > > <https://reviews.apache.org/r/4563/diff/1/?file=97495#file97495line213> > > > > This needs to be temp.begin() and temp.end(), since it might be > > changing. Please just kill add_begin, add_end. not needed. discussed offline and included a comment. > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/common/values.cpp, lines 251-252 > > <https://reviews.apache.org/r/4563/diff/1/?file=97495#file97495line251> > > > > Consider: > > > > const Value::Range& current = ranges->range(i); > > > > if (range.begin() <= current.begin() && ...) done > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/common/values.cpp, line 260 > > <https://reviews.apache.org/r/4563/diff/1/?file=97495#file97495line260> > > > > None of the continue statements are necessary. fixed > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/tests/resources_tests.cpp, lines 288-291 > > <https://reviews.apache.org/r/4563/diff/1/?file=97496#file97496line288> > > > > Please use comparisons which do "set" equality. fixed > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/tests/resources_tests.cpp, lines 406-411 > > <https://reviews.apache.org/r/4563/diff/1/?file=97496#file97496line406> > > > > Please use comparisons which do "set" equality. fixed > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/tests/resources_tests.cpp, line 267 > > <https://reviews.apache.org/r/4563/diff/1/?file=97496#file97496line267> > > > > Kind of redundant! fixed > On 2012-03-29 23:30:52, Benjamin Hindman wrote: > > src/tests/resources_tests.cpp, line 272 > > <https://reviews.apache.org/r/4563/diff/1/?file=97496#file97496line272> > > > > Newline. fixed - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4563/#review6550 ----------------------------------------------------------- On 2012-03-29 23:15:54, Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4563/ > ----------------------------------------------------------- > > (Updated 2012-03-29 23:15:54) > > > Review request for mesos and Bill Farner. > > > Summary > ------- > > Fixed both coalesce() and remove() > > > Diffs > ----- > > src/common/resources.hpp f367896 > src/common/values.cpp e6f1a85 > src/tests/resources_tests.cpp 7a68ed9 > > Diff: https://reviews.apache.org/r/4563/diff > > > Testing > ------- > > GLOG_v=1 bin/mesos-tests.sh -v --gtest_filter="*Ranges*" > > > Thanks, > > Vinod > >
