Skip to content

Implement ranges::transform #941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
CaseyCarter merged 3 commits into from
Jul 17, 2020
Merged

Implement ranges::transform #941

CaseyCarter merged 3 commits into from
Jul 17, 2020

Conversation

@miscco
Copy link
Contributor

@miscco miscco commented Jun 30, 2020

This implements the ranges::transform algorithm.

I need to track down just one more strange bug in the smoke test. Maybe a second pair of eyes helps.

I need to say the whole ranges machinery is intricate indeed.

@miscco miscco requested a review from a team as a code owner June 30, 2020 14:34
@miscco
Copy link
Contributor Author

miscco commented Jun 30, 2020

This is also one of the times where you just want clang-format to do its magic

@miscco miscco force-pushed the ranges_transform branch from b88051b to b89aa5a Compare June 30, 2020 14:51
miscco
miscco commented Jun 30, 2020
View reviewed changes
miscco
miscco commented Jun 30, 2020
View reviewed changes
miscco
miscco commented Jun 30, 2020
View reviewed changes
CaseyCarter
CaseyCarter reviewed Jun 30, 2020
View reviewed changes
CaseyCarter
CaseyCarter reviewed Jun 30, 2020
View reviewed changes
CaseyCarter
CaseyCarter reviewed Jun 30, 2020
View reviewed changes
@miscco miscco force-pushed the ranges_transform branch 2 times, most recently from 31397b1 to 83869e1 Compare June 30, 2020 20:44
@StephanTLavavej StephanTLavavej added the cxx20 C++20 feature label Jun 30, 2020
StephanTLavavej
StephanTLavavej requested changes Jun 30, 2020
View reviewed changes
@miscco miscco force-pushed the ranges_transform branch from 1603285 to dc69ae6 Compare July 1, 2020 07:32
miscco
miscco commented Jul 1, 2020
View reviewed changes
Copy link
Contributor Author

@miscco miscco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some serious reconsideration about certain life choices I got it to work. There are some questions open from my side

@miscco miscco force-pushed the ranges_transform branch 2 times, most recently from cbed41a to 4c56e06 Compare July 1, 2020 10:43
@CaseyCarter CaseyCarter mentioned this pull request Jul 2, 2020
Closed
@miscco miscco force-pushed the ranges_transform branch 2 times, most recently from 2aafddb to 46c28fc Compare July 8, 2020 17:36
@miscco
Copy link
Contributor Author

miscco commented Jul 8, 2020

Rebased and collapsed dopn

@StephanTLavavej StephanTLavavej added the ranges C++20/23 ranges label Jul 8, 2020
CaseyCarter
CaseyCarter suggested changes Jul 9, 2020
View reviewed changes
@miscco miscco force-pushed the ranges_transform branch 2 times, most recently from 2b10edb to da50860 Compare July 12, 2020 08:27
StephanTLavavej
StephanTLavavej requested changes Jul 13, 2020
View reviewed changes
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; I have very minor nitpicks.

@miscco miscco force-pushed the ranges_transform branch from da50860 to aed017e Compare July 13, 2020 10:48
@miscco
Copy link
Contributor Author

miscco commented Jul 13, 2020

Rebased, squashed and fixed

@CaseyCarter
Copy link
Contributor

CaseyCarter commented Jul 13, 2020

Rebased, squashed and fixed

For future reference: rebasing effectively makes incremental review impossible. It's generally preferable to merge master into the branch rather than rebasing the branch onto master, and we'll squash all the commits when we merge anyway.

StephanTLavavej
StephanTLavavej approved these changes Jul 13, 2020
View reviewed changes
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll push two more commas.

@CaseyCarter CaseyCarter self-assigned this Jul 16, 2020
@CaseyCarter CaseyCarter merged commit 002cdc6 into microsoft:master Jul 17, 2020
@CaseyCarter
Copy link
Contributor

CaseyCarter commented Jul 17, 2020

Thanks for helping to transform the STL into a Library with Ranges!

@CaseyCarter CaseyCarter removed their assignment Jul 17, 2020
@miscco miscco deleted the ranges_transform branch July 21, 2020 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+1 more reviewer

@CaseyCarter CaseyCarter CaseyCarter approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cxx20 C++20 feature ranges C++20/23 ranges

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@miscco @CaseyCarter @StephanTLavavej