Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-24 Thread Cong Wang


> On Feb. 24, 2016, 7:29 p.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/future.hpp, line 791
> > 
> >
> > * Mesos currently uses `foreach` from stout rather than 
> > range-based`for`.
> > * Mesos generally prefers explicit types than `auto`.
> > 
> > ```
> > foreach (Future future, futures) {
> >   ...
> > }
> > ```
> > 
> > Here and below.
> 
> Cong Wang wrote:
> Is that a strict rule? At least 
> src/tests/hierarchical_allocator_tests.cpp uses for(auto...) too.
> 
> Michael Park wrote:
> Yeah, those shouldn't have made it into the codebase for consistency. The 
> migration strategy is being discussed/tracked here: 
> [MESOS-3214](https://issues.apache.org/jira/browse/MESOS-3214).

OK. I will use foreach before we migrate to C++11 for loop.


- Cong


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


On Feb. 24, 2016, 7:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 24, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>



Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-24 Thread Michael Park


> On Feb. 24, 2016, 7:29 p.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/future.hpp, line 791
> > 
> >
> > * Mesos currently uses `foreach` from stout rather than 
> > range-based`for`.
> > * Mesos generally prefers explicit types than `auto`.
> > 
> > ```
> > foreach (Future future, futures) {
> >   ...
> > }
> > ```
> > 
> > Here and below.
> 
> Cong Wang wrote:
> Is that a strict rule? At least 
> src/tests/hierarchical_allocator_tests.cpp uses for(auto...) too.

Yeah, those shouldn't have made it into the codebase for consistency. The 
migration strategy is being discussed/tracked here: 
[MESOS-3214](https://issues.apache.org/jira/browse/MESOS-3214).


- Michael


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


On Feb. 24, 2016, 7:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 24, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>



Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-24 Thread Benjamin Bannier

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




3rdparty/libprocess/include/process/future.hpp (line 791)


Why do we need to create a copy here now?


- Benjamin Bannier


On Feb. 24, 2016, 8:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 24, 2016, 8:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>



Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-24 Thread Michael Park


> On Feb. 24, 2016, 7:30 p.m., Vinod Kone wrote:
> > LGTM. Though I'm not sure why we used iterators instead of foreach in the 
> > first place. cc @mcypark

Because it was written before `foreach` was introduced to stout. The last of 
the `for` loops here was authored 2011-10-31 and `foreach` was introduced 
2012-07-11.


- Michael


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


On Feb. 24, 2016, 7:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 24, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>



Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-24 Thread Cong Wang


> On Feb. 24, 2016, 7:29 p.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/future.hpp, line 791
> > 
> >
> > * Mesos currently uses `foreach` from stout rather than 
> > range-based`for`.
> > * Mesos generally prefers explicit types than `auto`.
> > 
> > ```
> > foreach (Future future, futures) {
> >   ...
> > }
> > ```
> > 
> > Here and below.

Is that a strict rule? At least src/tests/hierarchical_allocator_tests.cpp uses 
for(auto...) too.


- Cong


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


On Feb. 24, 2016, 7:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 24, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>



Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-24 Thread Vinod Kone

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


Ship it!




LGTM. Though I'm not sure why we used iterators instead of foreach in the first 
place. cc @mcypark

- Vinod Kone


On Feb. 24, 2016, 7:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 24, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>



Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-24 Thread Michael Park

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



Thanks for the cleanup!


3rdparty/libprocess/include/process/future.hpp (line 791)


* Mesos currently uses `foreach` from stout rather than range-based`for`.
* Mesos generally prefers explicit types than `auto`.

```
foreach (Future future, futures) {
  ...
}
```

Here and below.


- Michael Park


On Feb. 24, 2016, 7:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 24, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>



Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-24 Thread Cong Wang

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

(Updated Feb. 24, 2016, 7:19 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


Changes
---

rebase


Repository: mesos


Description
---

Used C++11 for loop to iterate std list/set.


Diffs (updated)
-

  3rdparty/libprocess/include/process/future.hpp 
77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 

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


Testing
---

make check


Thanks,

Cong Wang



Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-10 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43461]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 11, 2016, 12:38 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 11, 2016, 12:38 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>



Review Request 43461: Used C++11 for loop to iterate std list/set.

2016-02-10 Thread Cong Wang

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

Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


Repository: mesos


Description
---

Used C++11 for loop to iterate std list/set.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 

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


Testing
---

make check


Thanks,

Cong Wang