Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Alex Rukletsov]

Folks,

Sorry for not communicating it properly, I should have discarded the patch
actually. As Ben already mentioned, we had agreed to rework it and maybe
split into multiple patches for clarity. However, I haven't published them
yet.

Best,
Alex
On 23 Feb 2016 18:35, "Bernd Mathiske"  wrote:

>
>
> > On Feb. 23, 2016, 9:33 a.m., Ben Mahler wrote:
> > > Apologies, did you check with Alex prior to committing? We talked
> about this change recently but we didn't publish the comments, sorry that
> it wasn't clear! I reverted it for now, since we didn't really like the
> empty map constants as class members. We also needed to look into whether
> using empty initializer lists inline worked instead of this change.
>
> Alex is on vacation and I found this lingering. Since it looks a lot like
> an improvement, I went ahead after reading it all and testing it. No
> problem, we can progress further from either version.
>
>
> - Bernd
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/#review120338
> ---
>
>
> On Jan. 28, 2016, 5:12 a.m., Alexander Rukletsov wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/41950/
> > ---
> >
> > (Updated Jan. 28, 2016, 5:12 a.m.)
> >
> >
> > Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and
> Joris Van Remoortere.
> >
> >
> > Repository: mesos
> >
> >
> > Description
> > ---
> >
> > Changes made:
> > - empty resource map promoted to a const class field;
> > - removed variable numeric suffixes where appropriate;
> > - added const where appropriate.
> >
> >
> > Diffs
> > -
> >
> >   src/tests/hierarchical_allocator_tests.cpp
> f18e6eb10572b0f5b8bbff338384d9406f6ad62b
> >
> > Diff: https://reviews.apache.org/r/41950/diff/
> >
> >
> > Testing
> > ---
> >
> > On Mac OS 10.10.4:
> >
> > `make check`
> >
> > `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh
> --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`
> >
> >
> > Thanks,
> >
> > Alexander Rukletsov
> >
> >
>
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Bernd Mathiske]

> On Feb. 23, 2016, 9:33 a.m., Ben Mahler wrote:
> > Apologies, did you check with Alex prior to committing? We talked about 
> > this change recently but we didn't publish the comments, sorry that it 
> > wasn't clear! I reverted it for now, since we didn't really like the empty 
> > map constants as class members. We also needed to look into whether using 
> > empty initializer lists inline worked instead of this change.

Alex is on vacation and I found this lingering. Since it looks a lot like an 
improvement, I went ahead after reading it all and testing it. No problem, we 
can progress further from either version.


- Bernd


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


On Jan. 28, 2016, 5:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 28, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
> Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> f18e6eb10572b0f5b8bbff338384d9406f6ad62b 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check` 
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Ben Mahler]

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



Apologies, did you check with Alex prior to committing? We talked about this 
change recently but we didn't publish the comments, sorry that it wasn't clear! 
I reverted it for now, since we didn't really like the empty map constants as 
class members. We also needed to look into whether using empty initializer 
lists inline worked instead of this change.

- Ben Mahler


On Jan. 28, 2016, 1:12 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 28, 2016, 1:12 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
> Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> f18e6eb10572b0f5b8bbff338384d9406f6ad62b 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check` 
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Jan. 28, 2016, 9:12 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 28, 2016, 9:12 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
> Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> f18e6eb10572b0f5b8bbff338384d9406f6ad62b 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check` 
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Alexander Rukletsov]

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

(Updated Jan. 28, 2016, 1:12 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Changes made:
- empty resource map promoted to a const class field;
- removed variable numeric suffixes where appropriate;
- added const where appropriate.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
f18e6eb10572b0f5b8bbff338384d9406f6ad62b 

Diff: https://reviews.apache.org/r/41950/diff/


Testing
---

On Mac OS 10.10.4:

`make check` 

`GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`


Thanks,

Alexander Rukletsov

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Alexander Rukletsov]

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

(Updated Jan. 22, 2016, 10:51 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
Van Remoortere.


Changes
---

Rebased; more cleanups.


Repository: mesos


Description
---

Changes made:
- empty resource map promoted to a const class field;
- removed variable numeric suffixes where appropriate;
- added const where appropriate.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

Diff: https://reviews.apache.org/r/41950/diff/


Testing (updated)
---

On Mac OS 10.10.4:

`make check` 

`GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`


Thanks,

Alexander Rukletsov

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41950]

Failed command: ./support/apply-review.sh -n -r 41950

Error:
 2016-01-22 03:35:47 URL:https://reviews.apache.org/r/41950/diff/raw/ 
[24032/24032] -> "41950.patch" [1]
error: patch failed: src/tests/hierarchical_allocator_tests.cpp:1803
error: src/tests/hierarchical_allocator_tests.cpp: patch does not apply

- Mesos ReviewBot


On Jan. 22, 2016, 1:20 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 22, 2016, 1:20 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.5
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Alexander Rukletsov]

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

(Updated Jan. 22, 2016, 1:20 a.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van Remoortere.


Repository: mesos


Description
---

Changes made:
- empty resource map promoted to a const class field;
- removed variable numeric suffixes where appropriate;
- added const where appropriate.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

Diff: https://reviews.apache.org/r/41950/diff/


Testing
---

`make check` on Mac OS 10.10.5


Thanks,

Alexander Rukletsov

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Jan. 12, 2016, 1:34 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 12, 2016, 1:34 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.5
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41936, 41937, 41947, 41948, 41938, 41939, 41949, 41648, 41950]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 12, 2016, 9:34 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 12, 2016, 9:34 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.5
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 一月 12, 2016, 9:34 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated 一月 12, 2016, 9:34 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.5
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Joerg Schad]

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



src/tests/hierarchical_allocator_tests.cpp (line 223)


Should we maybe make the name more descriptive? EMPTY sounds very 
general


- Joerg Schad


On Jan. 5, 2016, 11:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 5, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.5
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Alexander Rukletsov]

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

(Updated Jan. 12, 2016, 9:34 p.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van Remoortere.


Changes
---

Rebased; addressed issues.


Repository: mesos


Description
---

Changes made:
- empty resource map promoted to a const class field;
- removed variable numeric suffixes where appropriate;
- added const where appropriate.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

Diff: https://reviews.apache.org/r/41950/diff/


Testing
---

`make check` on Mac OS 10.10.5


Thanks,

Alexander Rukletsov

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41936]

Failed command: ./support/apply-review.sh -n -r 41936

Error:
 2016-01-12 16:30:26 URL:https://reviews.apache.org/r/41936/diff/raw/ 
[23873/23873] -> "41936.patch" [1]
error: patch failed: src/tests/master_quota_tests.cpp:1088
error: src/tests/master_quota_tests.cpp: patch does not apply

- Mesos ReviewBot


On Jan. 5, 2016, 11:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 5, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.5
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 41950: Cleaned up hierarchical allocator tests.

[Alexander Rukletsov]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van Remoortere.


Repository: mesos


Description
---

Changes made:
- empty resource map promoted to a const class field;
- removed variable numeric suffixes where appropriate;
- added const where appropriate.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

Diff: https://reviews.apache.org/r/41950/diff/


Testing
---

`make check` on Mac OS 10.10.5


Thanks,

Alexander Rukletsov