> On Aug. 6, 2014, 9:38 a.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 375
> > <https://reviews.apache.org/r/24343/diff/2/?file=653521#file653521line375>
> >
> >     Option<size_t> capacity;
> >     if (limit_.has_capacity()) {
> >       capacity = limit_.capacity();
> >     }
> >     
> >     should work due to implicit conversion and is much clearer.

ok I agree. thanks.
I didn't remove the static_cast because it's more explicit and it might be a 
narrowing conversion. I added check for this case. 


> On Aug. 6, 2014, 9:38 a.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 394
> > <https://reviews.apache.org/r/24343/diff/2/?file=653521#file653521line394>
> >
> >     see above

done


> On Aug. 6, 2014, 9:38 a.m., Dominic Hamon wrote:
> > src/master/master.hpp, line 756
> > <https://reviews.apache.org/r/24343/diff/2/?file=653520#file653520line756>
> >
> >     it seems to me this could have some unit tests created outside of 
> > master tests. I'd really like to see that.

It's a good idea but for this review it's no longer applicable because the 
logic has been moved to Master::visit().
I agree in the future we should strive to test individual methods and in fact 
refactor long methods in Master. But in this case it proved to be difficult.


- Jiang Yan


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


On Aug. 6, 2014, 4:13 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2014, 4:13 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-1578
>     https://issues.apache.org/jira/browse/MESOS-1578
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207 
>   src/examples/load_generator_framework.cpp 
> 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4 
>   src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797 
>   src/master/master.cpp a925a936bc1cd878f1c9ff63a9dcc4ee808ec7a5 
>   src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31 
>   src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472 
> 
> Diff: https://reviews.apache.org/r/24343/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Reply via email to