User Details
- User Since
- May 10 2019, 9:08 AM (212 w, 1 d)
Today
Rebased and adjusted to upstream changes.
LGTM after addressing the review comments.
LGTM!
I didn't do an in depth review, I mainly looked at the code out of curiosity.
LGTM modulo 2 nits.
Rebased and address review comments.
Thanks for the review!
In general the approach seems fine, but I want to do some testing with this patch.
Especially to see what happens when the regenerate_expected_results flag is toggled.
Thu, Jun 1
Thanks for the review!
Thanks for working on these improvements! LGTM!
LGTM!
Now with approval selected ;-)
LGTM!
In general I'm happy, but I would like to see whether custom_container_iterator can be removed.
Could you add some information in the commit message why this is a good idea?
Wed, May 31
LGTM, thanks for fixing this.
I mainly glossed over it. I'm not to familiar with the details of these internals of ranges.
Did we add documentation for this new feature?
LGTM!
LGTM after undoing the formatting changes and the CI passes.
Can you make two followup commits
- format all the new files, I don't like to do that is the same commit, that makes tracking changes in git harder
- modernize the code _LIBCPP_INLINE_VISIBILITY -> _LIBCPP_HIDE_FROM_ABI, _VSTD -> std, typedef -> using
LGTM!
Tue, May 30
Thanks for catching me copy-pasting from the wrong place.
Addresses review comments.
Thanks for the reviews. I'll add release notes before committing.
Addresses review comments.
Sun, May 28
CI fixes
Adds an extra test.
Rebased on top of D151223.
Sat, May 27
Thanks for working on this! I didn't compare the code with the Standard. A few minor nits.
Fri, May 26
Not looked at the patch closely but I wonder about the usage of the pragma and their scope.
- Rebased to fix CI
- Fixes and modernizes some tests
Thu, May 25
Addresses review comments.
Wed, May 24
I'm happy, but since Louis had some remarks I leave the final approval to him.
LGTM, if possible I still would like benchmarks, but not a blocker.
@tahonermann when you have time can you have a look?
LGTM, thanks!
LGTM!
I like the change, but want to see a green CI before looking closer at it.
Would it make sense to add benchmarks? The number in D151274 look quite impressive.
Thanks, LGTM!
I've created D151344 @glandium @hans @thakis I really would appreciate when you can test the patch locally to avoid another revert round.
Tue, May 23
Rebased.