You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2017/08/15 07:00:49 UTC

Review Request 61638: Implemented LinkedHashMap copy construction and assignment.

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
-------

The defaults being generated were incorrect. No code is relying
on them today, this was discovered when writing new code.


Diffs
-----

  3rdparty/stout/include/stout/linkedhashmap.hpp 35684e46480761c9b8325525377ca3ca9707990c 
  3rdparty/stout/tests/linkedhashmap_tests.cpp 267022e3ea601300083a41cc559dfa0adbc9074b 


Diff: https://reviews.apache.org/r/61638/diff/1/


Testing
-------

Added a test.


Thanks,

Benjamin Mahler


Re: Review Request 61638: Implemented LinkedHashMap copy construction and assignment.

Posted by Benjamin Mahler <bm...@apache.org>.

> On Aug. 22, 2017, 11:54 p.m., Vinod Kone wrote:
> > 3rdparty/stout/tests/linkedhashmap_tests.cpp
> > Lines 207 (patched)
> > <https://reviews.apache.org/r/61638/diff/1/?file=1797256#file1797256line207>
> >
> >     Would this test fail without your fix above? Maybe you want to update the original after the copy is done to ensure the copy is not affected?

Updated the test to ensure the old code looks at the invalid iterators and crashes.


- Benjamin


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


On Aug. 23, 2017, 7:16 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61638/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2017, 7:16 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The defaults being generated were incorrect. No code is relying
> on them today, this was discovered when writing new code.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/linkedhashmap.hpp 35684e46480761c9b8325525377ca3ca9707990c 
>   3rdparty/stout/tests/linkedhashmap_tests.cpp 267022e3ea601300083a41cc559dfa0adbc9074b 
> 
> 
> Diff: https://reviews.apache.org/r/61638/diff/2/
> 
> 
> Testing
> -------
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 61638: Implemented LinkedHashMap copy construction and assignment.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61638/#review183555
-----------------------------------------------------------




3rdparty/stout/tests/linkedhashmap_tests.cpp
Lines 207 (patched)
<https://reviews.apache.org/r/61638/#comment259573>

    Would this test fail without your fix above? Maybe you want to update the original after the copy is done to ensure the copy is not affected?


- Vinod Kone


On Aug. 15, 2017, 7 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61638/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2017, 7 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The defaults being generated were incorrect. No code is relying
> on them today, this was discovered when writing new code.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/linkedhashmap.hpp 35684e46480761c9b8325525377ca3ca9707990c 
>   3rdparty/stout/tests/linkedhashmap_tests.cpp 267022e3ea601300083a41cc559dfa0adbc9074b 
> 
> 
> Diff: https://reviews.apache.org/r/61638/diff/1/
> 
> 
> Testing
> -------
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 61638: Implemented LinkedHashMap copy construction and assignment.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61638/#review183673
-----------------------------------------------------------


Fix it, then Ship it!





3rdparty/stout/tests/linkedhashmap_tests.cpp
Lines 214 (patched)
<https://reviews.apache.org/r/61638/#comment259741>

    copy paste the TODO here as well?


- Vinod Kone


On Aug. 23, 2017, 7:16 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61638/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2017, 7:16 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The defaults being generated were incorrect. No code is relying
> on them today, this was discovered when writing new code.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/linkedhashmap.hpp 35684e46480761c9b8325525377ca3ca9707990c 
>   3rdparty/stout/tests/linkedhashmap_tests.cpp 267022e3ea601300083a41cc559dfa0adbc9074b 
> 
> 
> Diff: https://reviews.apache.org/r/61638/diff/2/
> 
> 
> Testing
> -------
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 61638: Implemented LinkedHashMap copy construction and assignment.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61638/
-----------------------------------------------------------

(Updated Aug. 23, 2017, 7:16 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
-------

Updated the test to ensure the old code crashed.


Repository: mesos


Description
-------

The defaults being generated were incorrect. No code is relying
on them today, this was discovered when writing new code.


Diffs (updated)
-----

  3rdparty/stout/include/stout/linkedhashmap.hpp 35684e46480761c9b8325525377ca3ca9707990c 
  3rdparty/stout/tests/linkedhashmap_tests.cpp 267022e3ea601300083a41cc559dfa0adbc9074b 


Diff: https://reviews.apache.org/r/61638/diff/2/

Changes: https://reviews.apache.org/r/61638/diff/1-2/


Testing
-------

Added a test.


Thanks,

Benjamin Mahler