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

Optimize Plugin Repository Handling Process #106

Open
gounthar opened this issue Jul 17, 2024 · 3 comments
Open

Optimize Plugin Repository Handling Process #106

gounthar opened this issue Jul 17, 2024 · 3 comments
Labels
enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted good first issue Good for newcomers

Comments

@gounthar
Copy link
Collaborator

What feature do you want to see added?

Current Situation

We currently fork and clone the plugin repository locally before applying any recipes. This approach has several drawbacks:

  1. Wastes time and bandwidth
  2. Creates unnecessary forks, cluttering users' GitHub accounts
  3. Requires manual cleanup of unused forks

Issues

  1. Creating forks for plugins where recipes make no changes is inefficient
  2. Manual removal of unused forks is time-consuming (e.g., 15 minutes spent removing unchanged forks)
  3. Even with bot accounts, unnecessary forks are disrespectful of users' resources

Proposed Solution

Implement a more efficient process:

  1. Clone the upstream repository directly
  2. Create a local branch
  3. Apply recipes
  4. If changes are made:
    a. Commit changes
    b. Fork the repository
    c. Add the fork as a remote
    d. Push the modified branch to the fork

Benefits

  1. Reduces unnecessary forks
  2. Saves time and bandwidth
  3. Keeps users' GitHub accounts cleaner
  4. More respectful of users' resources

Considerations

  • This approach may seem counterintuitive at first
  • Implementation might be more complex

Request for Feedback

What are your thoughts on this proposed process? Are there any potential drawbacks or challenges we should consider?

Next Steps

  1. Evaluate the feasibility of this new approach
  2. Discuss potential implementation challenges
  3. If agreed upon, create a plan for implementing and testing this new process

By adopting this optimized approach, we can significantly improve the efficiency and user-friendliness of our tool while being more considerate of our users' resources.

Upstream changes

No response

Are you interested in contributing this feature?

No response

@gounthar gounthar added the enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Jul 17, 2024
@jonesbusy
Copy link
Collaborator

This is partially fixed with https://github.com/jenkinsci/plugin-modernizer-tool/pull/140/files#diff-79e555d036d6ee53eae22db579ed66b43b07ff65c1226cfede96396438156f68R317 which prevent forking in dry-run-mode

But the order of operation need to be optimized

plugin.fork(ghService);
if (config.isRemoveLocalData()) {
    LOG.info("Removing local data for plugin: {} at {}", plugin, plugin.getLocalRepository());
    plugin.removeLocalData();
}

must be done after

plugin.verify(mavenInvoker);
if (plugin.hasErrors()) {
    LOG.warn(
            "Skipping plugin {} due to verification errors after modernization. Check logs for more details.",
            plugin.getName());
    return;
}

Perhaps some code need to be adapted for (example the public void fetch(Plugin plugin) assume to fetch on fork if not in dry-run mode. So perhaps it should always fetch from original repo and just ensure the remote is set to the fork before pushing changes)

@jonesbusy
Copy link
Collaborator

This is still valid. We should not fork until we need to open a PR

@jonesbusy jonesbusy added the good first issue Good for newcomers label Sep 7, 2024
@CodexRaunak
Copy link

CodexRaunak commented Dec 24, 2024

@jonesbusy I am interested in working on this issue and have been exploring the related codebase. Here's what I've done so far:

1)Added a verification check before forking the plugin to ensure only valid plugins proceed further in the workflow (fixing the order).
2)Updated the fetch function to always fetch from the original repository even in non dry run state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants