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



src/Makefile.am
<https://reviews.apache.org/r/33040/#comment131872>

    I don't see this file?



src/linux/routing/queueing/fq_codel.hpp
<https://reviews.apache.org/r/33040/#comment131871>

    I don't see any code in the header uses hashmap?



src/linux/routing/queueing/fq_codel.hpp
<https://reviews.apache.org/r/33040/#comment131870>

    WHy add one more blank line? Please revert.



src/linux/routing/queueing/fq_codel.cpp
<https://reviews.apache.org/r/33040/#comment131875>

    Please move this above `operator ==` and add a new line between them.



src/linux/routing/queueing/fq_codel.cpp
<https://reviews.apache.org/r/33040/#comment131869>

    Why add one more blank line?



src/linux/routing/queueing/handle.hpp
<https://reviews.apache.org/r/33040/#comment131868>

    Why remove this blank line?



src/linux/routing/queueing/ingress.hpp
<https://reviews.apache.org/r/33040/#comment131874>

    I don't see hashmap being used?



src/linux/routing/queueing/ingress.hpp
<https://reviews.apache.org/r/33040/#comment131873>

    Please revert.



src/linux/routing/queueing/ingress.cpp
<https://reviews.apache.org/r/33040/#comment131877>

    Ditto.



src/linux/routing/queueing/ingress.cpp
<https://reviews.apache.org/r/33040/#comment131878>

    To be consistent, do you want to get rid of the "ingress" here as well. You 
can make 'name' a static method of Discipline and use 
`ingress::Discipline::name()` consistently.



src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/33040/#comment131880>

    Thanks! Could you please add a test for this?



src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/33040/#comment131879>

    s/if/If/
    
    Also, add a period at the end of the sentense.



src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/33040/#comment131883>

    Please revert.



src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/33040/#comment131885>

    Why this change? What if in the future we have two  versions of fq_codel 
qdiscs. Only using the name to distinguish seems to be insufficient. Could you 
please revert this change?



src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/33040/#comment131881>

    Please revert.



src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/33040/#comment131882>

    Please revert as well.



src/tests/routing_tests.cpp
<https://reviews.apache.org/r/33040/#comment131886>

    Add a period at the end of the sentense.



src/tests/routing_tests.cpp
<https://reviews.apache.org/r/33040/#comment131887>

    Could you please remove these prints?



src/tests/routing_tests.cpp
<https://reviews.apache.org/r/33040/#comment131888>

    Ditto on removal.


- Jie Yu


On April 24, 2015, 5:46 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33040/
> -----------------------------------------------------------
> 
> (Updated April 24, 2015, 5:46 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
> 
> 
> Bugs: mesos-2332
>     https://issues.apache.org/jira/browse/mesos-2332
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Expose qdisc statistics from libnl
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/linux/routing/queueing/fq_codel.hpp 
> 4f67ab7d64afea96a07dfcf36769a9c667749a00 
>   src/linux/routing/queueing/fq_codel.cpp 
> 02ad8df7814c0e549a9ca9aef39777684e6abdcb 
>   src/linux/routing/queueing/handle.hpp 
> 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 
>   src/linux/routing/queueing/ingress.hpp 
> b323a7f6daed828327d6d9e9740df81582e0ba2b 
>   src/linux/routing/queueing/ingress.cpp 
> 47c73376097d70819defdee31a6d1e446df6b8ba 
>   src/linux/routing/queueing/internal.hpp 
> 7c6c4d3d960b9a4bf44dcf482212317522353d69 
>   src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba 
> 
> Diff: https://reviews.apache.org/r/33040/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Reply via email to