Skip to content

Extracted _Choice<T>._Strategy to variable #1426

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
StephanTLavavej merged 6 commits into from
Nov 7, 2020
Merged

Extracted _Choice<T>._Strategy to variable #1426

StephanTLavavej merged 6 commits into from
Nov 7, 2020

Conversation

@futuarmo
Copy link
Contributor

@futuarmo futuarmo commented Nov 4, 2020

According to issue #1419 I've extracted Choice._Strategy to separate variable in <ranges> abd <xutility>

@futuarmo futuarmo requested a review from a team as a code owner November 4, 2020 13:33
miscco
miscco reviewed Nov 4, 2020
View reviewed changes
Copy link
Contributor

@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.

Thanks a lot. There are some minimal whitespace problems

miscco
miscco reviewed Nov 4, 2020
View reviewed changes
CaseyCarter
CaseyCarter approved these changes Nov 4, 2020
View reviewed changes
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

I see one occurrence of wonky formatting, which I'll fix by applying my suggestion, and then this LGTM.

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Nov 4, 2020
CaseyCarter
CaseyCarter reviewed Nov 4, 2020
View reviewed changes
@CaseyCarter CaseyCarter linked an issue Nov 4, 2020 that may be closed by this pull request
<ranges>, <xutility>: Extract _Strategy to simplify code #1419
Closed
@CaseyCarter
Copy link
Contributor

CaseyCarter commented Nov 4, 2020 •
edited
Loading

According to issue #1419 I've extracted Choice._Strategy to separate variable in <ranges> abd <xutility>

@futuarmo I've linked up the issue manually, but FYI for next time if you include the GitHub magic words "Fixes #1419" in your commit message then GitHub will link the issue to the PR automatically.

@futuarmo
Copy link
Contributor Author

futuarmo commented Nov 4, 2020

@CaseyCarter ok, I've got it, thanks

StephanTLavavej
StephanTLavavej approved these changes Nov 4, 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.

😸

@StephanTLavavej StephanTLavavej changed the title Extracted Choice<T>._Strategy to variable Extracted _Choice<T>._Strategy to variable Nov 4, 2020
@StephanTLavavej StephanTLavavej added the ranges C++20/23 ranges label Nov 4, 2020
StephanTLavavej added a commit to StephanTLavavej/STL that referenced this pull request Nov 5, 2020
@StephanTLavavej
microsoftGH-1426 Extracted _Choice<T>._Strategy to variable
3ab55b1
@StephanTLavavej StephanTLavavej self-assigned this Nov 5, 2020
StephanTLavavej added a commit to StephanTLavavej/STL that referenced this pull request Nov 5, 2020
@StephanTLavavej
microsoftGH-1426 Extracted _Choice<T>._Strategy to variable
43ac3f6
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Nov 7, 2020

Fixed a merge conflict in <ranges>; the recently added/moved views were too much for git to handle. Dropped attempted damage to _Drop_while_fn::operator() and manually applied this PR's change to _Reverse_fn::operator().

@StephanTLavavej StephanTLavavej merged commit 10d8e33 into microsoft:master Nov 7, 2020
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Nov 7, 2020

Thanks for improving the clarity of this ranges machinery! 😸

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

+2 more reviewers

@CaseyCarter CaseyCarter CaseyCarter approved these changes

@miscco miscco miscco approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

enhancement Something can be improved ranges C++20/23 ranges

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

<ranges>, <xutility>: Extract _Strategy to simplify code

4 participants

@futuarmo @CaseyCarter @StephanTLavavej @miscco