Applied recipes AddOrModernizeJenkinsFile, AddDependencyCheck, AddPluginsBom, UpgradeParentVersion, RemoveDependencyVersionOverride, RemoveExtraMavenProperties, UpgradeToJava17, AddOrModernizeJenkinsFile, AddDependencyCheck, UpgradeToRecommendCoreVersion, MigrateJUnitTestCase #178
Conversation
…ginsBom, UpgradeParentVersion, RemoveDependencyVersionOverride, RemoveExtraMavenProperties, UpgradeToJava17, AddOrModernizeJenkinsFile, AddDependencyCheck, UpgradeToRecommendCoreVersion, MigrateJUnitTestCase
WDYM? There was no such conversion. (Anyway I would not be eager to accept it if there were—JUnit 4 works fine from my PoV, and I have no interest in JUnit 5.) |
| /* | ||
| See the documentation for more options: | ||
| https://github.com/jenkins-infra/pipeline-library/ | ||
| */ buildPlugin( | ||
| useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests |
There was a problem hiding this comment.
Useless.
| /* | |
| See the documentation for more options: | |
| https://github.com/jenkins-infra/pipeline-library/ | |
| */ buildPlugin( | |
| useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests | |
| buildPlugin( | |
| useContainerAgent: true, |
There was a problem hiding this comment.
Archetype must be adapted then : https://github.com/jenkinsci/archetypes/blob/master/common-files/Jenkinsfile#L1a
At some point we just need to decide to keep such comment or not.
There was a problem hiding this comment.
- the rewrite is missing
forkCount - would be nicer if the rewrite did not touch a file which was materially the same (ignoring whitespace)
- the proposed rewrite had mangled whitespace after the comment
There was a problem hiding this comment.
FWIW I think the comment is fine in the archetype, because that is deliberately intended as a skeleton for a brand-new plugin showing you some things you might consider doing and explaining what they mean if you have never encountered them before. Seems less helpful in the OpenRewrite proposed text. Would be less intrusive to have a single-line comment like
// Reference: https://github.com/jenkins-infra/pipeline-library/blob/f26089381c825325666e3f2f1b31e35e9f63c2c7/README.adoc#buildplugin(Here I am using the permalink since the docs might well be rearranged in the future. Or just the repo link would suffice I guess.)
| <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo> | ||
| <maven.compiler.release>17</maven.compiler.release> |
There was a problem hiding this comment.
No! This should never be done this way in a plugin repo. If the dep is 2.479.x and the parent is 5.x then this is automatic.
| </properties> | ||
| <dependencyManagement> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>io.jenkins.tools.bom</groupId> | ||
| <artifactId>bom-2.440.x</artifactId> | ||
| <version>3435.v238d66a_043fb_</version> | ||
| <artifactId>bom-2.452.x</artifactId> |
There was a problem hiding this comment.
This should anyway be using the newer idiom with a shared property for LTS line.
There was a problem hiding this comment.
jenkinsci/archetypes#738
jenkins-infra/plugin-modernizer-tool#368
Need most likely to be fixed on https://github.com/openrewrite/rewrite-jenkins/blob/main/src/main/java/org/openrewrite/jenkins/AddPluginsBom.java
The recipe is already handling the mismatch of jenkins.version and bom. But since we moved to the new property the recipe must be adapted
|
@gounthar I think there are some issues ^^^ |
|
Indeed. |
Hello
build-token-rootdevelopers!This is an automated pull request created by the Jenkins Plugin Modernizer tool. The tool has applied the following recipes to modernize the plugin:
Add Jenkinsfile
io.jenkins.tools.pluginmodernizer.AddOrModernizeJenkinsFile
Add dependency check
io.jenkins.tools.pluginmodernizer.AddDependencyCheck
Add plugins BOM
io.jenkins.tools.pluginmodernizer.AddPluginsBom
Upgrade parent version
io.jenkins.tools.pluginmodernizer.UpgradeParentVersion
Remove dependency version override
io.jenkins.tools.pluginmodernizer.RemoveDependencyVersionOverride
Remove extra maven properties
io.jenkins.tools.pluginmodernizer.RemoveExtraMavenProperties
Migrate from Java 11 to Java 17
io.jenkins.tools.pluginmodernizer.UpgradeToJava17
Add Jenkinsfile
io.jenkins.tools.pluginmodernizer.AddOrModernizeJenkinsFile
Add dependency check
io.jenkins.tools.pluginmodernizer.AddDependencyCheck
Upgrade to latest recommended core version and ensure the bom is matching the core version
io.jenkins.tools.pluginmodernizer.UpgradeToRecommendCoreVersion
Convert JUnit 4 TestCase to JUnit Jupiter
io.jenkins.tools.pluginmodernizer.MigrateJUnitTestCase