RFC, NFC: refactor ranges to be nested templates #535
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
So the nested type was my initial plan for how to do this. I discussed with @davidchisnall and he suggested the default and inner using. I implemented both and went with the using due to the smaller diff. I do find the occasional <> annoying, and I think the nested type is slightly clearer code. But I'm not sure if it is worth changing. |
This way, we don't have to specify a Parent when we're just interested in Pipe-ing things together. We could have called these inner classes Apply and left the Pipe implementation alone, but it's probably better to call them Type and adjust the Pipe code.
nwf
force-pushed
the
202205-range-nested-templates
branch
from
b1c3003 to
89fc430
Compare
mjp41
approved these changes
Jun 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This way, we don't have to specify a Parent when we're just interested in
Pipe-ing things together.
We could have called these inner classes Apply and left the Pipe implementation
alone, but it's probably better to call them Type and adjust the Pipe code.
This commit is absolutely best reviewed ignoring whitespace changes!
This commit also conflicts, for trivial reasons, with #530. I would be tempted to wait until that has landed and then commute this back across that if we want this.