mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 33792: Extend hashmap to support custom equality and hash
Date Mon, 11 May 2015 22:47:21 GMT

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


Thanks! Could you split the patch between the implementation and the test? The implementation
looks pretty good so would like to give a ship it on that before I review the test. Much appreciated
:)


3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
<https://reviews.apache.org/r/33792/#comment134234>

    We wrap lines of code beyond 80 characters, but this fits on one line:
    
    ```
    >>> len('class hashmap : public boost::unordered_map<Key, Value, Hash, Pred>')
    67
    ```
    
    Can you undo the line wrapping here?



3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
<https://reviews.apache.org/r/33792/#comment134235>

    Have you looked at the style guide? We try to avoid this style of wrapping because it
is "jagged", can you wrap as follows:
    
    ```
    boost::unordered_map<Key, Value, Hash, Pred>::emplace(
        it->first, it->second);
    ```



3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
<https://reviews.apache.org/r/33792/#comment134236>

    Fits on one line:
    
    ```
    >>> len('    boost::unordered_map<Key, Value, Hash, Pred>::erase(key);')
    61
    ```



3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
<https://reviews.apache.org/r/33792/#comment134237>

    Can you wrap as follows:
    
    ```
    boost::unordered_map<Key, Value, Hash, Pred>::insert(
        std::pair<Key, Value>(key, value));
    ```



3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp
<https://reviews.apache.org/r/33792/#comment134238>

    Add an extra line here to be consistent with the rest of this file


- Ben Mahler


On May 9, 2015, 3:36 p.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33792/
> -----------------------------------------------------------
> 
> (Updated May 9, 2015, 3:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Ben Mahler.
> 
> 
> Bugs: MESOS-328
>     https://issues.apache.org/jira/browse/MESOS-328
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Extend hashmap to support custom equality and hash
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 24dc369ec271ec2f35449e6ccf49c5b829ca6ce8

>   3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp e8a932e5474bf2ba1a93a945ff9bc61fb5146c02

> 
> Diff: https://reviews.apache.org/r/33792/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>


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