Reworked pipeline to introduce phases for ordering #2444
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
* Removed nested Pipelin.Subsystems * Removed tests due to immutable ValueSubsystem * Fixed bugs found in testing: * Kind needs to be passed to PipelinePhase constructor, Subsystem may be null so is not userful for retrieving * Phase list needs to be added in Pipeline constructor so is available to empty pipeline * PipelinePhrase needs to be a reference type as changes need to be available * Logic for getting specific subsystems didn't account for nulls correclty * Teardown now consistently runs Execute * Invocation and Validation are phases
This avoids a type check and moves a runtime error to compiler time for setting the main phase subsystem, which must be of the correct type r null. Currently base and generic are in the same file to facilitate review (not having all the code show up as new in commits)
Also updated dotnet#2410 to describe
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.
Updated #2410 to describe new approach.
PipleinePhases handle the ordering of subsystems. The underlying assumptions are:
Also in this PR:
ResponseSubsystemand made it non-replaceable for nowTeardownmore consistentlyInvocationSubsystemandValidationSubsystemValueSubsystema non-replaceable subsystemNote: I had to set this aside for several weeks and had a code loss git-astrophe that may have resulted in code loss as I picked it back up - so @mhutch and others, if this does not appear to be the design we discussed, I look forward to that discussion.