You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/10/02 20:15:53 UTC
Review Request 26273: Define reverseforeach from Boost.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26273/
-----------------------------------------------------------
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos-git
Description
-------
Define reverseforeach from Boost.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 0739d6360464873533af07e67f6beb7804af43aa
Diff: https://reviews.apache.org/r/26273/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 26273: Define reverseforeach from Boost.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26273/#review55250
-----------------------------------------------------------
Can you link in the motivating patch?
It seems nice to keep a single foreach looping construct, and instead transform what we're looping over. In this case, I believe you wanted to reverse a vector?
```
vector<Isolator*> isolators);
foreach (Isolator* isolator, reversed(isolators)) {
// reversed() is same as python built-in: https://docs.python.org/2/library/functions.html#reversed
}
foreach (Isolator* isolator, boost::adaptors::reverse(isolators) {
...
}
```
Because what happens when we want to additional iteration constraints?
```
foreach (string word, reversed(sorted(words))) {
// ...
}
```
This approach is composable, as opposed to new looping constructs:
```
unique_reversed_foreach (string word, words) {
// ...
}
```
- Ben Mahler
On Oct. 2, 2014, 6:15 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26273/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 6:15 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Define reverseforeach from Boost.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 0739d6360464873533af07e67f6beb7804af43aa
>
> Diff: https://reviews.apache.org/r/26273/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 26273: Define reverseforeach from Boost.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26273/#review55672
-----------------------------------------------------------
I am also against this because not every std container has reverse_iterator.
- Jie Yu
On Oct. 2, 2014, 6:15 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26273/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 6:15 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Define reverseforeach from Boost.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 0739d6360464873533af07e67f6beb7804af43aa
>
> Diff: https://reviews.apache.org/r/26273/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 26273: Define reverseforeach from Boost.
Posted by "Timothy St. Clair" <ts...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26273/#review55244
-----------------------------------------------------------
Ship it!
Ship It!
- Timothy St. Clair
On Oct. 2, 2014, 6:15 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26273/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 6:15 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Define reverseforeach from Boost.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 0739d6360464873533af07e67f6beb7804af43aa
>
> Diff: https://reviews.apache.org/r/26273/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 26273: Define reverseforeach from Boost.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26273/#review55235
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26273]
All tests passed.
- Mesos ReviewBot
On Oct. 2, 2014, 6:15 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26273/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 6:15 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Define reverseforeach from Boost.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 0739d6360464873533af07e67f6beb7804af43aa
>
> Diff: https://reviews.apache.org/r/26273/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 26273: Define reverseforeach from Boost.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26273/#review55477
-----------------------------------------------------------
I agree with Ben. I think we prefer adaptors such as `boost::adaptors::reverse` here so we can compose them. Another thing to consider is looking ahead to adopting more C++11 constructs into Mesos. It's a trivial transformation to modernize the `foreach` macro to `range-based for-loop`, but modernizing `reverseforeach` would be non-trivial as we need to also modify the container somehow. Using `foreach` + an adaptor would keep the transformation trivial.
- Michael Park
On Oct. 2, 2014, 6:15 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26273/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 6:15 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Define reverseforeach from Boost.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 0739d6360464873533af07e67f6beb7804af43aa
>
> Diff: https://reviews.apache.org/r/26273/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>