Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with Precondition documentation generation #250

Open
SimonVerhoeven opened this issue Nov 25, 2023 · 6 comments
Open

Issue with Precondition documentation generation #250

SimonVerhoeven opened this issue Nov 25, 2023 · 6 comments
Assignees
Labels
blocked When an issue can't be worked on right now bug Something isn't working

Comments

@SimonVerhoeven
Copy link
Contributor

For recipes such as https://docs.openrewrite.org/recipes/java/spring/boot3/enablevirtualthreads which contain a precondition (in this case org.openrewrite.java.search.HasJavaVersion the recipe list shows Precondition bellwether which links to https://github.com/openrewrite/rewrite-docs/blob/master/reference/recipes/config/declarativerecipe$preconditionbellwether.md rather than the expected HasJavaVersion recipe + link.

@timtebeek
Copy link
Contributor

Thanks for pointing this out! You've found a neat implementation detail around how we handle preconditions in declarative recipes; the message you saw originates here. Since the class is package private the best I could do for now is recognize such recipes by their message, and then skip them.

@timtebeek timtebeek self-assigned this Nov 25, 2023
@timtebeek
Copy link
Contributor

@timtebeek timtebeek reopened this Nov 25, 2023
@github-project-automation github-project-automation bot moved this from Done to Backlog in OpenRewrite Nov 25, 2023
@timtebeek
Copy link
Contributor

So the issue here is that the bellwether is shown on the Yaml tab:
image

That's an implementation detail; not something for folks to implement. Might mean we need to move this to
https://github.com/openrewrite/rewrite-recipe-markdown-generator

There it shows up in the yaml tab as

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.config.DeclarativeRecipe$PreconditionBellwether
  - org.openrewrite.java.spring.AddSpringProperty

@timtebeek
Copy link
Contributor

Compare that to how the recipe is actually implemented:
https://github.com/openrewrite/rewrite-spring/blob/b76e03437143632ec9d42ffc0a5e7af2b88f11c9/src/main/resources/META-INF/rewrite/spring-boot-32.yml#L63-L73

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

mike-solomon added a commit to openrewrite/rewrite-recipe-markdown-generator that referenced this issue Jan 23, 2024
timtebeek pushed a commit to openrewrite/rewrite-recipe-markdown-generator that referenced this issue Jan 24, 2024
@timtebeek
Copy link
Contributor

As discussed on the above PR: it would be better to show the configured precondition & recipe arguments in the bellwether. So right now what we show is:

Current

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

@mike-solomon
Copy link
Contributor

As mentioned in this issue, I believe that changes will need to be made in rewrite itself (specifically RecipeDescriptor) to allow for Preconditions to be made available to the docs.

@mike-solomon mike-solomon added the blocked When an issue can't be worked on right now label Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked When an issue can't be worked on right now bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

3 participants