Implementing a Dry-Run Mode

gounthar commented 1 month ago

What feature do you want to see added?

Current Situation

Currently, our tool immediately applies recipes and creates a pull request (PR) without any intermediate steps.

Problem

This approach can lead to several issues:

Inability to review changes before PR creation
Potential for sending unfinished or problematic PRs to plugin maintainers
Risk of creating out-of-date PRs

Proposed Solution: Dry-Run Mode

We should implement a dry-run feature similar to other command-line tools like updatecli. This would involve two main modes:

1. Dry-Run Mode (similar to updatecli's `diff`)

Run recipes without creating a PR
Display potential code changes
Allow users to review modifications
Provide an opportunity to build and test the plugin locally
Add an option to build the plugin after the changes

2. Apply Mode (similar to updatecli's `apply`)

Create the PR with the changes

Benefits

Quality Control: Users can review and refine changes before submitting PRs
Respect for Maintainers: Reduces the risk of sending incomplete or problematic PRs
Flexibility: Allows for local testing and validation before PR creation
Alignment with Best Practices: Follows the pattern of established tools in the ecosystem

Example Scenario

Recently, an automated PR was sent to a plugin maintainer that was not up-to-date with the main branch code. A dry-run mode could have prevented this situation by allowing for a pre-submission review.

Conclusion

Implementing a dry-run mode would significantly improve our tool's usability and reliability. It would provide users with more control over the modernization process and help maintain good relationships with plugin maintainers by ensuring higher quality PRs.

Upstream changes

No response

Are you interested in contributing this feature?

No response

sridamul commented 1 month ago

Hey @gounthar, we already have --dry-run option (Available CLI options) which generates patch files and no PRs

gounthar commented 1 month ago

My bad! We can close, then!

gounthar commented 1 month ago

Reopening because having an output that lists the changes could be helpful.

java -jar plugin-modernizer-cli/target/jenkins-plugin-modernizer-999999-SNAPSHOT.jar --dry-run --plugins badge-plugin,build-timestamp-plugin --recipes AddPluginsBom,AddCodeOwner
Picked up JAVA_TOOL_OPTIONS: -XX:+UseContainerSupport -XX:ActiveProcessorCount=1
Starting Plugin Modernizer 
Picked up JAVA_TOOL_OPTIONS: -XX:+UseContainerSupport -XX:ActiveProcessorCount=1
Plugins: [badge-plugin, build-timestamp-plugin] 
Recipes: [AddPluginsBom, AddCodeOwner] 
GitHub owner: gounthar 
Forking and cloning badge-plugin locally 
Repository already forked to personal account gounthar 
Invoking clean phase for plugin: badge-plugin 
Invoking rewrite plugin for plugin: badge-plugin 
Skipping commit and pull request creation for badge-plugin 
Forking and cloning build-timestamp-plugin locally 
Repository already forked to personal account gounthar 
Invoking clean phase for plugin: build-timestamp-plugin 
Invoking rewrite plugin for plugin: build-timestamp-plugin 
Skipping commit and pull request creation for build-timestamp-plugin

**Dry Run enabled**

⚠ The line #4, matched by the keyword "FROM" and the matcher "gitpod/workspace-full", is changed from "FROM gitpod/workspace-full:2024-07-10-08-22-03" to "FROM gitpod/workspace-full:2024-07-14-17-19-51".
⚠ - changed lines [4] of file "/tmp/updatecli/github/jenkins-docs/quickstart-tutorials/.gitpod/Dockerfile"

Please see https://github.com/jenkins-docs/quickstart-tutorials/actions/runs/9941995064/job/27462405629#step:5:133 .

jonesbusy commented 1 month ago

I will leave this issue open. Dry run is implemented but can be optimized

For example I don't think we should run rewrite in dry run mode (to create the patch) but always apply changes on the local repo.

The dry-run doesn't fork the repo anymore but clones/fetch from original repo
The dry-run doesn't commit changes anymore (so it means we can easily do a diff on the local repo to debug)
The dry-run compile the plugin after rewrite. Which is useless for now because no changes is never made to the local repo

Conclusion we need never should running the rewrite plugin in dry-mode (or have a separate CLI option different from --dry-run)

@sridamul What do you think about this approach ?

sridamul commented 1 month ago

Overall seems like a good idea.

Conclusion we need never should running the rewrite plugin in dry-mode (or have a separate CLI option different from --dry-run)

Maybe we can change the option to something like --rewrite-dry-run instead of removing the existing option.

jonesbusy commented 3 weeks ago

Closed by https://github.com/jenkinsci/plugin-modernizer-tool/pull/169 feel free to open a new issue for more improvement for dry-run mode

jenkinsci / plugin-modernizer-tool