search

openrewrite / rewrite-recipe-markdown-generator

Utility that generates OpenRewrite recipe documentation in markdown format for all recipes on the classpath.
Apache License 2.0
2 stars 7 forks source link

Remove bellwether from generated yaml #99

Closed mike-solomon closed 8 months ago

mike-solomon commented 8 months ago

Related to issue: https://github.com/openrewrite/rewrite-docs/issues/250

You can see there is no bellwether in the yaml recipe list generated:

image

timtebeek commented 8 months ago

While this would hide the bellwether (good) is would be better to show the configured precondition in the bellwether. So right now what we show is:

Current

All the issues of the next section + showing internal class bellwether. image

With this change

No bellwether (good), no precondition (missed opportunity, it's in the bellwether), no arguments to AddSpringProperty, (leading to a broken yaml recipe)

type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.spring.boot3.EnableVirtualThreads
displayName: Enable Virtual Threads on Java 21
description: Set `spring.threads.virtual.enabled` to `true` in `application.properties` or `application.yml`.
recipeList:
  - org.openrewrite.java.spring.AddSpringProperty

Ideal

As per the source

type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.spring.boot3.EnableVirtualThreads
displayName: Enable Virtual Threads on Java 21
description: Set `spring.threads.virtual.enabled` to `true` in `application.properties` or `application.yml`.
preconditions:
  - org.openrewrite.java.search.HasJavaVersion:
      version: 21.X
recipeList:
  - org.openrewrite.java.spring.AddSpringProperty:
      property: spring.threads.virtual.enabled
      value: true
timtebeek commented 8 months ago

This is an improvement already, but still shows broken config to the user in that recipe list tab. We can still merge this already though, but adding that context.

mike-solomon commented 8 months ago

Ah I see -- thanks for clarifying again. I'm going to close this PR as it doesn't really do what we want.

mike-solomon commented 8 months ago

Agh my bad @timtebeek didn't see your follow up comment before closing haha. I'm gonna poke around with it a bit more first and may reopen if I can't find anything.

mike-solomon commented 8 months ago

After stepping through the debugger, I don't believe we currently have access to be able to output the preconditions or see anything beyond what we currently have. It's possible I missed something, but I couldn't find HasJavaVersion anywhere in the EnableVirtualThreads recipe in the debugger.

I think we would need to update rewrite itself and change the RecipeDescriptor class to include a preconditions variable or something.

I'll go ahead and reopen this PR as it feels like it's worthwhile to merge in as is.