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

Document maven command line to invoke recipes #44

Closed
MarkEWaite opened this issue Aug 26, 2023 · 6 comments · Fixed by #45
Closed

Document maven command line to invoke recipes #44

MarkEWaite opened this issue Aug 26, 2023 · 6 comments · Fixed by #45
Labels
enhancement New feature or request

Comments

@MarkEWaite
Copy link
Contributor

What problem are you trying to solve?

The OpenRewrite documentation for the Jenkins recipes includes examples that allow me to run a recipe from the Maven command line, without modifying my pom.xml file. I'd like to apply the hudson.Util.getPastTimeString migration to the Jenkins basic branch build strategies plugin but I was unable to determine the command line options that are needed to apply that recipe.

Describe the solution you'd like

A sample Maven command line that will invoke that recipe or one of the other recipes in this repository would be enough to let me explore further.

Have you considered any alternatives or workarounds?

I could do the text replacement myself, but I wanted to try OpenRewrite with a recipe that I know is needed on a plugin that I maintain.

Additional context

Thanks so much for your work on OpenRewrite recipes for Jenkins.

Are you interested in contributing this feature to OpenRewrite?

I'm happy to help with documentation once I understand the basics.

@MarkEWaite MarkEWaite added the enhancement New feature or request label Aug 26, 2023
@timtebeek
Copy link
Contributor

Hi @MarkEWaite , I've just created a new release to make that recipe available to you most easily. Instructions will be similar to this one:
https://docs.openrewrite.org/recipes/jenkins/addpluginsbom

mvn -U org.openrewrite.maven:rewrite-maven-plugin:run \
  -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-jenkins:RELEASE \
  -Drewrite.activeRecipes=org.openrewrite.jenkins.AddPluginsBom

Hope that helps!

@timtebeek
Copy link
Contributor

And of course you can also run it through app.moderne.io if that suits you. Saves you a bit of a wait and helps create the PR.

@MarkEWaite
Copy link
Contributor Author

Hi @MarkEWaite , I've just created a new release to make that recipe available to you most easily. Instructions will be similar to this one: https://docs.openrewrite.org/recipes/jenkins/addpluginsbom

Thanks @timtebeek! Unfortunately, that recipe does not make any change when applied to the basic branch build strategies plugin.

I also tried the command:

mvn -U org.openrewrite.maven:rewrite-maven-plugin:run   \
  -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-jenkins:RELEASE  \
  -Dorg.openrewrite.jenkins.migrate.hudson.UtilGetPastTimeStringToGetTimeSpanString

That ran without error but did not make any change to the plugin source code. I was expecting a change on lines 213 and 214 of src/main/java/jenkins/branch/buildstrategies/basic/TagBuildStrategyImpl.java

Is there something else that I need to do?

@timtebeek
Copy link
Contributor

Sorry to hear that didn't yet have the desired effect. I'm going to tag @sghill here since he might know more.
Otherwise I can have a look when I return Wednesday.

@sghill
Copy link
Collaborator

sghill commented Aug 26, 2023

Thanks for the mention Tim!

@MarkEWaite Looks like this command is super close - it's just missing the rewrite.activeRecipes= prefix on the system prop. I tried this out just now and it produced the changes you mentioned:

$ mvn -U org.openrewrite.maven:rewrite-maven-plugin:run \
      -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-jenkins:RELEASE \
      -Drewrite.activeRecipes=org.openrewrite.jenkins.migrate.hudson.UtilGetPastTimeStringToGetTimeSpanString
[WARNING] Changes have been made to src/main/java/jenkins/branch/buildstrategies/basic/TagBuildStrategyImpl.java by:
[WARNING]     org.openrewrite.jenkins.migrate.hudson.UtilGetPastTimeStringToGetTimeSpanString
[WARNING]         org.openrewrite.java.ChangeMethodName: {methodPattern=hudson.Util getPastTimeString(long), newMethodName=getTimeSpanString, matchOverrides=false, ignoreDefinition=true}
[WARNING] Please review and commit the results.
diff --git a/src/main/java/jenkins/branch/buildstrategies/basic/TagBuildStrategyImpl.java b/src/main/java/jenkins/branch/buildstrategies/basic/TagBuildStrategyImpl.java
index 5a9af36..b31ba38 100644
--- a/src/main/java/jenkins/branch/buildstrategies/basic/TagBuildStrategyImpl.java
+++ b/src/main/java/jenkins/branch/buildstrategies/basic/TagBuildStrategyImpl.java
@@ -210,8 +210,8 @@ public class TagBuildStrategyImpl extends BranchBuildStrategy {
     @Override
     public String toString() {
         return "TagBuildStrategyImpl{" + "atLeast="
-                + (atLeastMillis >= 0L ? Util.getPastTimeString(atLeastMillis) : "n/a") + ", atMost="
-                + (atMostMillis >= 0L ? Util.getPastTimeString(atMostMillis) : "n/a") + '}';
+                + (atLeastMillis >= 0L ? Util.getTimeSpanString(atLeastMillis) : "n/a") + ", atMost="
+                + (atMostMillis >= 0L ? Util.getTimeSpanString(atMostMillis) : "n/a") + '}';
     }
 
     /**

Great suggestion for documenting how to run this on the command line, and thank you for trying out the recipes!

@MarkEWaite
Copy link
Contributor Author

Thanks for the mention Tim!

@MarkEWaite Looks like this command is super close - it's just missing the rewrite.activeRecipes= prefix on the system prop. I tried this out just now and it produced the changes you mentioned:

$ mvn -U org.openrewrite.maven:rewrite-maven-plugin:run \
      -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-jenkins:RELEASE \
      -Drewrite.activeRecipes=org.openrewrite.jenkins.migrate.hudson.UtilGetPastTimeStringToGetTimeSpanString

That's great! My apologies that I didn't deduce that on my own. It makes perfect sense that the recipe property name needs to be provided as well as the value. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants