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

Add force branch name settings #519

Open
wants to merge 5 commits into
base: intellij2020.3
Choose a base branch
from

Conversation

Al3xCalibur
Copy link

One main idea of Gerrit's flow is to push on master, with custom git behaviours.
Thus on remote, it is very exceptional to work on another branch.
But locally, it still makes sense to use branches and to push on master.
Especially allows for a nice presentation on Gerrit with the "pushed together commits"

That's why it makes sense for the plugin to force the main branch based on the gitreview file.
But it only covers the case of a single local repo/gerritPushTargetPanel.
For multiple ones, we must type "master" in the branch input each time we push using a local branch.
It happens very often in my case.

Given I'm not sure everyone wants that behaviour (and I tried to understand the codebase), I've added a specific setting

It forces the usage of git review branch name by default, even with local branches
It helps the classic workflow to always push on master, other branches are exceptional
@uwolfer
Copy link
Owner

uwolfer commented Feb 13, 2025

@Al3xCalibur Sorry, I somehow missed this PR. Just looked into it now.

This new option only works with a .gitreview file, right? Wouldn't it make sense to fall back to the repo default branch if no such file is present?

@Al3xCalibur
Copy link
Author

Hey, thanks for the answer!

Yes, it is based on the similar behaviour for a single repo

It makes complete sense to fallback on the default branch

@uwolfer
Copy link
Owner

uwolfer commented Feb 13, 2025

It makes complete sense to fallback on the default branch

Do you think you could add this to your PR? I tested the current status, and code looks good as well.

@Al3xCalibur
Copy link
Author

Yes, I can do it this evening (I've already started to re-setup my PC for the project)

@Al3xCalibur
Copy link
Author

I've tried to get the default branch for a repo, but I can't find a reasonable way (aka with git4idea)
Ref for the main idea from cli: https://superuser.com/questions/1718677/find-the-default-branch-name-for-a-git-repository

@uwolfer
Copy link
Owner

uwolfer commented Feb 14, 2025

Thanks for looking into that. In that case, I would suggest that you add a hint to the settings label that it only works in case .gitreview file is set up.

@Al3xCalibur
Copy link
Author

I finally found a way
Git4idea exposes a wrapper for cli within a static instance.
So I can simply apply the regex from the linked url, I'll just check subtleties with gerrit

Retrieving the remote default branch name requires the CLI
In that case, it does some network call so it can be quite long
That's why the swing worker as been introduced
@Al3xCalibur
Copy link
Author

Al3xCalibur commented Feb 14, 2025

Hey, I've pushed a proposition.
I'm not sure if we want to keep a reference on the GitRepository, worst case I can use the root virtual file and get the project later.

And all the ways to get the remote default branch name requires a bit of IO, but it is probably fine in a worker and as an opt-in option

I've tested locally seems fine on a normal git repo, but I can test it on gerrit on Monday

@uwolfer
Copy link
Owner

uwolfer commented Feb 15, 2025

Thanks, it really looks the remote call is required in this case. I have the feeling that SwingWorker is not the optimal solution for IntelliJ, unfortunately IntelliJ has for a lot of stuff different APIs which should be used. You can search for GitVcs.runInBackground(new Task.Backgroundable in this codebase, this is probably what you want here?

@Al3xCalibur
Copy link
Author

Thanks, I will have a look at this methods
I'm clearly a beginner on UI related stuffs for plugins

It creates tasks on idea, rather than custom swing code
@uwolfer
Copy link
Owner

uwolfer commented Feb 19, 2025

Code looks good, thanks!

Did you test the following cases:

  1. option disabled, behavior should be as without your change
  2. option enabled, .gitreview present with default branch -> should use that branch and not request from remote
  3. option enabled, .gitreview not present -> should call remote and use that default branch (also test a case where the default branch is not named master / main)

@Al3xCalibur
Copy link
Author

I've tested options 2 and 3, but not 1
I'm quite confident it is fine, but I will do the extra test (probably on Friday)

Same limitation than push by default on gerrit
@Al3xCalibur
Copy link
Author

I've tried the option 1, it works fine
But there is the same limitation than push by default on gerrit (as expected, same implementation)
So I've added the same message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants