replace visibility handling in recipes with CLI-specific implementation - #144
replace visibility handling in recipes with CLI-specific implementation#144KamilPatora wants to merge 18 commits into
Conversation
| name: pl.allegro.tech.allwrite.recipes.YamlPublicRecipe | ||
| displayName: Public YAML recipe | ||
| tags: | ||
| - visibility:public |
There was a problem hiding this comment.
public recipes have to declare group and action (not!) (borat reference)
# Conflicts: # allwrite-recipes/src/main/kotlin/pl/allegro/tech/allwrite/recipes/gradle/ChangeGradleDependency.kt # allwrite-recipes/src/main/kotlin/pl/allegro/tech/allwrite/recipes/spring/ChangeSpringBoot4WebServerTypes.kt
…nto refactor/cli-recipe-discovery
There was a problem hiding this comment.
Pull request overview
Replaces visibility-based recipe discovery with explicit CLI recipe metadata.
Changes:
- Adds
CliAllwriteRecipeand removesRecipeVisibility. - Makes
RecipeSource.findAll()return all recipes; CLI consumers filter by coordinates. - Updates recipes, tests, fixtures, completions, and documentation.
Reviewed changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
docs/recipes/index.md |
Documents new recipe helpers. |
docs/index.md |
Adds the CLI base class. |
docs/contributing.md |
Explains recipe discovery and authoring. |
docs/cli.md |
Updates ls behavior. |
allwrite-spi/.../RecipeVisibility.kt |
Removes visibility enum. |
allwrite-spi/.../RecipeMetadata.kt |
Removes visibility metadata. |
allwrite-spi/.../CliAllwriteRecipe.kt |
Adds CLI-specific recipe base. |
allwrite-spi/.../AllwriteScanningRecipe.kt |
Removes visibility parameters. |
allwrite-spi/.../AllwriteRecipe.kt |
Simplifies metadata and enables tag extension. |
allwrite-runtime/src/testFixtures/resources/META-INF/rewrite/yaml-public-recipe.yaml |
Uses CLI coordinate tags. |
allwrite-runtime/src/testFixtures/resources/META-INF/rewrite/yaml-internal-recipe.yaml |
Removes visibility tag. |
allwrite-runtime/src/testFixtures/kotlin/.../FakeRecipeSource.kt |
Updates fake discovery behavior and tags. |
allwrite-runtime/src/testFixtures/kotlin/.../FakePostProcessingRecipe.kt |
Removes visibility configuration. |
allwrite-runtime/src/testFixtures/kotlin/.../FakeCompositeRecipe.kt |
Removes visibility configuration. |
allwrite-runtime/src/testFixtures/kotlin/.../KotlinPublicRecipe.kt |
Adds CLI coordinate tags. |
allwrite-runtime/src/testFixtures/kotlin/.../KotlinInternalRecipe.kt |
Removes visibility tag. |
allwrite-runtime/src/testFixtures/java/.../JavaPublicRecipe.java |
Adds CLI coordinate tags. |
allwrite-runtime/src/testFixtures/java/.../JavaInternalRecipe.java |
Removes visibility tag. |
allwrite-runtime/src/test/kotlin/.../RecipeSourceSpec.kt |
Tests complete and CLI-filtered discovery. |
allwrite-runtime/src/test/kotlin/.../RecipeMetadataSpec.kt |
Tests CLI tags and validation. |
allwrite-runtime/src/test/kotlin/.../RecipeCoordinatesSpec.kt |
Tests CLI recipe identification. |
allwrite-runtime/src/main/kotlin/.../OpenrewriteRecipeSource.kt |
Returns all recipe descriptors. |
allwrite-recipes/src/test/kotlin/.../AddVersionCatalogDependencyReferenceTest.kt |
Adapts test recipe construction. |
allwrite-recipes/src/main/kotlin/.../YamlEntryHasValue.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../UnnestProperties.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../ReplaceStatusCodeValue.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../RenameTaskExecutorBean.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../RemoveAnnotatedMethod.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../PreconditionsAwareAddDependency.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../IsolatedSpringRecipe.kt |
Migrates to CliAllwriteRecipe. |
allwrite-recipes/src/main/kotlin/.../FindSpringProperty.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../DeleteSpringPropertyWithValue.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../DeleteSpringPropertyFromSpringAnnotations.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../DeleteSpringProperty.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../ChangeSpringPropertyKey.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../AddNonNullableTypeBoundsToSpringRepositories.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../FindProperties.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../ReplaceFactoryWithConstructor.kt |
Adapts base-class construction. |
allwrite-recipes/src/main/kotlin/.../IsolatedJavaRecipe.kt |
Migrates to CliAllwriteRecipe. |
allwrite-recipes/src/main/kotlin/.../ChangeRecordField.kt |
Adapts base-class construction. |
allwrite-recipes/src/main/kotlin/.../UpdateGradleDependency.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../ChangeGradleDependency.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/kotlin/.../AddGradleDependency.kt |
Removes visibility configuration. |
allwrite-recipes/src/main/java/.../RemoveUnusedImportsOfType.java |
Removes visibility tags. |
allwrite-completions/src/main/kotlin/.../GenerateCompletionsAnnotationProcessor.kt |
Limits completions to CLI recipes. |
allwrite-cli/src/test/kotlin/.../RecipeMatcherSpec.kt |
Tests non-CLI exclusion. |
allwrite-cli/src/test/kotlin/.../ListRecipesCommandSpec.kt |
Removes --all tests. |
allwrite-cli/src/test/kotlin/.../FailingPostProcessingRecipe.kt |
Removes visibility configuration. |
allwrite-cli/src/main/kotlin/.../RunWithDependabotCommand.kt |
Filters Dependabot discovery by CLI tags. |
allwrite-cli/src/main/kotlin/.../RecipeMatcher.kt |
Matches only CLI recipes. |
allwrite-cli/src/main/kotlin/.../ListRecipesCommand.kt |
Lists only CLI recipes and removes --all. |
allwrite-api/src/main/kotlin/.../RecipeSource.kt |
Simplifies the discovery API. |
allwrite-api/src/main/kotlin/.../RecipeCoordinates.kt |
Adds CLI detection and exact tag parsing. |
AGENTS.md |
Updates architectural documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 54 changed files in this pull request and generated no new comments.
Suppressed comments (3)
allwrite-runtime/src/testFixtures/kotlin/pl/allegro/tech/allwrite/runtime/fake/FakeRecipeSource.kt:19
findAll()now means “all descriptors” (as implemented byOpenrewriteRecipeSource), but this fake still removes every non-CLI recipe. That makes fake-backed tests diverge from production and causes the new matcher test to pass without exercisingRecipeMatcher's own filter because its non-CLI fixture is discarded here first. Return all mapped descriptors from the fake as well.
allwrite-cli/src/main/kotlin/pl/allegro/tech/allwrite/cli/application/RunWithDependabotCommand.kt:67- This filter prevents Dependabot from discovering recipes that have a
dependabot-artifacttag but intentionally lack CLIgroup/actiontags. That contradicts the newly documented FQN-only Dependabot support indocs/contributing.md:100-101, anddependabotArtifactsremains available on both non-CLI base classes. The later coordinate conversion and matcher also require CLI tags, so this path needs to select Dependabot recipes independently of CLI coordinates (or the documented/API support must be removed).
val recipes = recipeSource.findAll().filter { it.isCliRecipe() }
allwrite-cli/src/main/kotlin/pl/allegro/tech/allwrite/cli/application/ListRecipesCommand.kt:17
- The command now filters the output to CLI recipes only, but its user-facing help still says “Lists all recipes.” Update the help text so
allwrite ls --helpaccurately describes the changed behavior.
val recipes = recipeSource.findAll().filter { it.isCliRecipe() }.sortedBy { it.name }
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 56 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
allwrite-cli/src/main/kotlin/pl/allegro/tech/allwrite/cli/application/RunWithDependabotCommand.kt:72
- Non-CLI recipes are now executed solely because the artifact tag matches; their
from/totags are never checked against the Dependabot update. Since regular and scanning recipes still expose version metadata, a recipe for (for example) 2→3 will incorrectly run for an unrelated 1→2 bump. Apply the same version-range eligibility check before adding the descriptor name.
if (matchingRecipe != null && !matchingRecipe.isCliRecipe()) {
listOf(matchingRecipe.name)
| val matchingRecipe = update.findMatchingRecipe(recipes) | ||
| if (matchingRecipe != null && !matchingRecipe.isCliRecipe()) { | ||
| listOf(matchingRecipe.name) |
radoslaw-panuszewski
left a comment
There was a problem hiding this comment.
LGTM! (only one question)
| @@ -5,16 +5,13 @@ import org.openrewrite.ScanningRecipe | |||
| public abstract class AllwriteScanningRecipe<T : Any> @JvmOverloads public constructor( | |||
There was a problem hiding this comment.
What about CliAllwriteScanningRecipe?
Refactor Recipe Discovery
Summary
Replace the
RecipeVisibility.PUBLIC/INTERNALmodel with an explicitCliAllwriteRecipebase class.Changes
CliAllwriteRecipewith requiredgroupandactionvalues.RecipeSource.allwrite ls, recipe matching, and shell completions.allwrite ls --alloption.Expected Result
User-facing migrations remain available through friendly CLI commands, while internal and option-based recipes are hidden and should be used in migration code