fix: Add DelegatingRecipe to find underlying recipe for loading options #4286
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
…wrappers. Fixes a problem where RecipeDescriptor does not have the options on it when a recipe has preconditions.
sambsnyd
reviewed
Jun 25, 2024
Comment on lines
424
to
426
| public abstract static class DelegatingRecipe extends Recipe { | ||
| public abstract Recipe getDelegate(); | ||
| } |
There was a problem hiding this comment.
Any reason this is an abstract class rather than an interface? Seems like it could be an interface
There was a problem hiding this comment.
Because it extends recipe, which is an abstract class. We could remove the extends ofcourse, but I thought this made more sense
There was a problem hiding this comment.
I tried the inteface, it looks a little nicer, but it does not force the DelegatingRecipe to be a Recipe itself. Not sure which I like better. What do you think? (see last commit)
timtebeek
reviewed
Jun 27, 2024
| @@ -168,7 +168,7 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { | |||
|
|
|||
| @EqualsAndHashCode(callSuper = false) | |||
| @Value | |||
| static class BellwetherDecoratedRecipe extends Recipe { | |||
| static class BellwetherDecoratedRecipe extends Recipe implements DelegatingRecipe { | |||
There was a problem hiding this comment.
Should BellwetherDecoratedScanningRecipe below implement the same interface? Or a DelegatingScanningRecipe for that part?
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.
Fixes a problem where RecipeDescriptor does not have the options on it when a recipe has preconditions.
What's changed?
When a recipe is actually a delegate it can return that delegate to be used in option extraction.
What's your motivation?
Currently a recipe with preconditions will have no options on the descriptors of children
Have you considered any alternatives or workarounds?
Adding a specific check for the Bellwether wrapper, but I think this is cleaner.