Complete adoption into OpenRewrite GitHub Org

[timtebeek]

What problem are you trying to solve?

Because this project started out independently, it diverges from other OpenRewrite modules, making it slightly harder to maintain automatically.

Describe the solution you'd like

Ensure we use the code style, CI and release pipeline from OpenRewrite.

• Use the rewrite-build-gradle-plugin in build.gradle.kts
• Use openrewrite/.github for CI
• Use Java for tests
• Add @DocumentExample to tests for documentation
• Include module in recipe-markdown-generator
• Include module in recipe-bom
• Update README to include the usual blurps
• Move rewrite.yml out of root
• Drop duplicate LICENSE
• Use 8 for src/main and 17 for src/test, likely drop java-version
• Disassociate fork from rewrite-recipe-starter through GitHub support
• Add a category.yml to show recipes in docs & platform under Jenkins with a logo

Have you considered any alternatives or workarounds?

No; it's preferred for projects to look alike, such that we can maintain, release and document them with ease.

[timtebeek]

Most of the items above can be ticked off by looking at sibling modules, some such as the @DocumentExample are even available as recipe through Moderne, provided we get CI going first, and port the tests from Kotlin to Java first (I think).

[sghill]

@timtebeek it looks like the detach fork request needs to come from an OpenRewrite admin. I tried it here, but got this message.

Transcript:

• from_agent: Hi, @sghill.
• from_agent: A fork is a copy of a repository. Forking a repository allows you to freely experiment with changes without affecting the original project.
• from_agent: Are you looking to delete a fork or detach/extract a fork?
• from_agent: Detach - To create a new fork network containing only this repository. All of its sub-forks are re-parented and stay in the original fork network.
• from_agent: Extract - To move the fork and all its subforks into a new network. This repository becomes the new root. Delete Detach/Extract
• from_user: Detach/Extract
• from_agent: Please enter the owner/repository-name or the URL of the forked repository that you'd like detached/extracted
• from_user: openrewrite/rewrite-jenkins
• from_agent: Checking repository status, please wait...
• from_agent: It looks like either openrewrite/rewrite-jenkins doesn't exist or you are not an admin, only an admin can make this request.
• from_agent: All done. Thank you for using Virtual Assistant.

[timtebeek]

Thanks! I've put an application as well; last time around it took only a couple hours I believe, so thanks for the reminder and link to get this started. And great to already see so many commits tagging this issue; looking forward to what you'll do with it! :)

[timtebeek]

Back from my travels and realized I'd forgotten to add this module to the platform before I left; your tweet was a nice reminder! I've gone and deployed the latest snapshot to https://app.moderne.io/marketplace/org.openrewrite.jenkins such that you can now also run your Java recipes there easily; let me know if you'd ever want a new snapshot to be deployed; we typically do so weekly.

[sghill]

Thanks @timtebeek! Is slack the best place for a snapshot deployment request?

It looks like the platform already has the logo, so I added the category.yml and checked that task. Looking at the module adding tasks next.

I ran the DocumentExample recipe and was curious why the annotation wasn't added to DisableLocalResolutionForParentPomTest. Should I manually annotate the other tests?

Also had a question about recipe structuring -- should I keep the recipes separate by version? I was modeling this a bit like the spring boot recipes, where the 2.x to 3.x recipe would call all the intermediate recipes (2.1 to 2.2). I liked this for a couple reasons:

• recipes remain stable over time
• easy to compose the last upgrade into the current upgrade
• easy to see which parent pom required other recipes
• a very old plugin may be able to go partially up-to-date easily, but the full migration may be better split over future commits. I've mostly seen this so far with upgrading to the last working Java 8 version, and then making the Java 11 jump in a different migration.

I'm now a bit worried that we'll end up with hundreds of these recipes, since Jenkins tooling upgrades happen fast. So an alternative would be to maintain fewer, larger recipes (ModernizePlugin, UpgradeToLastJava8SupportedVersions).

[timtebeek]

Thanks @timtebeek! Is slack the best place for a snapshot deployment request?

Slack works and could be faster; I've gone ahead and deployed the latest snapshot just now!

It looks like the platform already has the logo, so I added the category.yml and checked that task. Looking at the module adding tasks next.

Awesome; merged those two PRs and crossed them off above. Thanks for adding the category.yml; helps feature the recipes in the appropriate place with some context.

I ran the DocumentExample recipe and was curious why the annotation wasn't added to DisableLocalResolutionForParentPomTest. Should I manually annotate the other tests?

That's odd indeed; I'd have expected it to be added there as well; I don't see any issues with ingestion or the test itself, and from the SelectRecipeExamples implementation can't work out why it wasn't applied. 🤔
Even ran it again with the same result for this project; did find and applied a couple other still, so thanks for the reminder!

For this project I guess we're going to either add some of those manually, or try to find and fix the issue with the recipe. :/

[timtebeek]

Also had a question about recipe structuring -- should I keep the recipes separate by version? I was modeling this a bit like the spring boot recipes, where the 2.x to 3.x recipe would call all the intermediate recipes (2.1 to 2.2). I liked this for a couple reasons:

• recipes remain stable over time
• easy to compose the last upgrade into the current upgrade
• easy to see which parent pom required other recipes
• a very old plugin may be able to go partially up-to-date easily, but the full migration may be better split over future commits. I've mostly seen this so far with upgrading to the last working Java 8 version, and then making the Java 11 jump in a different migration.

I'm now a bit worried that we'll end up with hundreds of these recipes, since Jenkins tooling upgrades happen fast. So an alternative would be to maintain fewer, larger recipes (ModernizePlugin, UpgradeToLastJava8SupportedVersions).

Good question; I don't know too much about the Jenkins internals, but I imagine you have a couple large bump versions, that have proven a hurdle in adoption, for instance by requiring more extensive changes, or upgrades to Java 8, 11, Jakarta, etc.. It could make sense then to treat those as "major" versions that each get their own yaml file, which can contain multiple documents to pull people up to that version, while chaining in any previous versions.

That way plugin developers are able pick from a number of well known reference points to pick up all changes up to that point, and migrate and review in steps. So rather than having potentially hundreds of individual recipes and files for each Jenkins version, instead pick a few reference points and group them that way.

Would that be an approach that sounds feasible here?

[timtebeek]

Looks like we ran through all the tasks set out in the initial issue above; all that's left then is to do an initial release I suppose, such that we can include this module with the next release of OpenRewrite and all it's modules. Are we ready for such a release? Or is there anything else you'd want to cover?

[sghill]

I'd like to try grouping by reference point as you suggested above, but that doesn't need to hold up an initial release if you'd prefer to do that sooner -- I see that rewrite-recipe-markdown-generator is failing due to not having an initial release.

[timtebeek]

I'll ping @mike-solomon to let him now we'd need to create a v0.2.0 tag of this repository before he can run the recipe-markdown-generator main branch again; that'll do for now I suppose. We can do that release at any time, and it'll pick up whatever the state is then. So you have a little while longer, as the docs were last updated Friday, and are on a rough weekly schedule.

We can close this issue after that initial release; and can of course continue the above discussions even after it's closed.

[sghill]

Sounds great! Could we get a snapshot deployed (whenever it's convenient) so I could see how some recent changes look on the platform?

I've made a few changes recently:

• Additional docs: displayNames, descriptions on declarative recipes, Option docs on java recipes
• Retired some unused, experimental recipes
• Tests and DocumentExample added for remaining recipes
• Recipe grouping by reference point - this looks much better in the output when I run it locally

[WARNING] Changes have been made to pom.xml by:
[WARNING]     org.openrewrite.jenkins.ModernizePlugin
[WARNING]         org.openrewrite.maven.ChangeParentPom: {oldGroupId=org.jenkins-ci.plugins, oldArtifactId=plugin, newVersion=latest.release, allowVersionDowngrades=false}
[WARNING]         org.openrewrite.jenkins.UpgradeVersionProperty: {key=jenkins.version, minimumVersion=2.375.4}
[WARNING]         org.openrewrite.jenkins.AddPluginsBom
[WARNING] Please review and commit the results.

Locally the changes are looking good. I've upgraded a couple plugins and have seen their builds pass. There are a couple nice-to-haves still on my list:

• CreateIndexJellyTest is disabled because the recipe is completing in 0 cycles
• This pom is reformatting every line to 2 spaces when I run the ModernizePlugin recipe, even if I activate a style I thought would force 4 spaces. Seems to be unique to this repo for some reason.

type: specs.openrewrite.org/v1beta/style
name: io.jenkins.plugins.style
styleConfigs:
  - org.openrewrite.xml.style.TabsAndIndentsStyle:
      useTabCharacter: false
      tabSize: 4
      indentSize: 4

[timtebeek]

Sounds great! Could we get a snapshot deployed (whenever it's convenient) so I could see how some recent changes look on the platform?

Changes look good to me; thanks again! Deployed the latest snapshot to https://app.moderne.io/marketplace/org.openrewrite.jenkins

I've made a few changes recently:

• Additional docs: displayNames, descriptions on declarative recipes, Option docs on java recipes
• Retired some unused, experimental recipes
• Tests and DocumentExample added for remaining recipes
• Recipe grouping by reference point - this looks much better in the output when I run it locally

[WARNING] Changes have been made to pom.xml by:
[WARNING]     org.openrewrite.jenkins.ModernizePlugin
[WARNING]         org.openrewrite.maven.ChangeParentPom: {oldGroupId=org.jenkins-ci.plugins, oldArtifactId=plugin, newVersion=latest.release, allowVersionDowngrades=false}
[WARNING]         org.openrewrite.jenkins.UpgradeVersionProperty: {key=jenkins.version, minimumVersion=2.375.4}
[WARNING]         org.openrewrite.jenkins.AddPluginsBom
[WARNING] Please review and commit the results.

Locally the changes are looking good. I've upgraded a couple plugins and have seen their builds pass. There are a couple nice-to-haves still on my list:

• CreateIndexJellyTest is disabled because the recipe is completing in 0 cycles

Not sure why those other three tests are failing, but the first noop test fails with a NPE that might also affect runs on projects that already contain a jelly file; here's what I'd suggest to prevent that NPE:

diff --git a/src/main/java/org/openrewrite/jenkins/CreateIndexJelly.java b/src/main/java/org/openrewrite/jenkins/CreateIndexJelly.java
index fa13391..11db884 100644
--- a/src/main/java/org/openrewrite/jenkins/CreateIndexJelly.java
+++ b/src/main/java/org/openrewrite/jenkins/CreateIndexJelly.java
@@ -52,7 +52,7 @@ public class CreateIndexJelly extends ScanningRecipe<CreateIndexJelly.Scanned> {
     public TreeVisitor<?, ExecutionContext> getVisitor(Scanned acc) {
         return Preconditions.check(acc.isJenkinsPlugin, new CreateTextFile(
                 acc.description,
-                acc.indexJellyPath.toString(),
+                acc.indexJellyPath,
                 true
         ).getVisitor());
     }
@@ -77,7 +77,7 @@ public class CreateIndexJelly extends ScanningRecipe<CreateIndexJelly.Scanned> {
                                 .orElse(pom.getMarkers().findFirst(MavenResolutionResult.class)
                                         .map(maven -> maven.getPom().getArtifactId())
                                         .orElseThrow(() -> new IllegalStateException("Expected to find an artifact id")));
-                        acc.indexJellyPath = sourceFile.getSourcePath().resolve("../src/main/resources/index.jelly").normalize();
+                        acc.indexJellyPath = sourceFile.getSourcePath().resolve("../src/main/resources/index.jelly").normalize().toString();
                         acc.isJenkinsPlugin = true;
                     }
                 }
@@ -89,6 +89,6 @@ public class CreateIndexJelly extends ScanningRecipe<CreateIndexJelly.Scanned> {
     static class Scanned {
         boolean isJenkinsPlugin;
         String description;
-        Path indexJellyPath;
+        String indexJellyPath;
     }
 }

• This pom is reformatting every line to 2 spaces when I run the ModernizePlugin recipe, even if I activate a style I thought would force 4 spaces. Seems to be unique to this repo for some reason.

type: specs.openrewrite.org/v1beta/style
name: io.jenkins.plugins.style
styleConfigs:
  - org.openrewrite.xml.style.TabsAndIndentsStyle:
      useTabCharacter: false
      tabSize: 4
      indentSize: 4

Hmm; we might need to pass that style in when ingesting; although we typically get away with detecting the style as long as it's consistent. In the case of that pom.xml file the elements under <properties> are indented with two spaces where I would have expected four. It might already help to make it consistent there, although I'm not a 100% certain that's enough. We can try it and otherwise look at alternatives such as passing in the style when ingesting.

[timtebeek]

With the release of https://github.com/openrewrite/rewrite-jenkins/releases/tag/v0.2.0 we can close this issue. Thanks again for starting this effort, and all the work done so far in the adoption, running recipes at scale, and spreading the word about your results. ❤️