Bug 509181 - checkout change branch when fetching patch set if the patch set is already fetched
Summary: checkout change branch when fetching patch set if the patch set is already fe...
Status: REOPENED
Alias: None
Product: EGit
Classification: Technology
Component: UI (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 513673
Blocks:
  Show dependency tree
 
Reported: 2016-12-13 17:32 EST by Jaxsun McCarthy Huggan CLA
Modified: 2017-05-18 08:04 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jaxsun McCarthy Huggan CLA 2016-12-13 17:32:47 EST
It's not uncommon for me to want to checkout the code from a review multiple times. When the "Fetch" button is clicked in the reviews editor it always attempts to fetch the patch set and create a new branch for it. It would be nice for it to offer to change to the patch set branch if it sees a branch with a conflicting name during fetch. If a user turns down this offer the previous workflow would resume allowing the user to checkout the patch set to a different branch.
Comment 1 Sam Davis CLA 2016-12-14 13:54:51 EST
The fetch dialog comes from EGit so I'm moving this there.
Comment 2 Jaxsun McCarthy Huggan CLA 2016-12-14 16:54:58 EST
Thanks Sam
Comment 3 Matthias Sohn CLA 2016-12-14 17:37:44 EST
EGit has no review editor so I am not sure what you are describing here.
I also don't understand what stops you from checking out the same branch multiple times.
What do you mean with a "conflicting branch".

Can you provide detailed steps to reproduce the problem you see ?
Comment 4 Jaxsun McCarthy Huggan CLA 2016-12-14 19:04:15 EST
Hi Matthias, this was initially reported to Mylyn, hence the description about the review editor.

Mylyn reviews uses the FetchGerritChangeWizard to checkout patch sets from gerrit reviews. If this wizard is invoked twice the second time it will open and tell the user that they already have a branch with the given refname. It would be nice if when a user hits this condition the wizard were to show a page prompting them to change to that branch.

I'm sure there are ways to reproduce this with just egit, but I will give some simple steps to reproduce via Mylyn since that is how I am familiar with this wizard.

Steps to reproduce:
* Open a gerrit review with Mylyn
* choose a patch set and click "Fetch..."
* complete the fetch wizard
* change to some other branch (eg. master)
* open the same review and click "Fetch..." on the same patch set
* the fetch wizard opens and warns the user that there is already a branch with the given name

I would be interested in contributing this functionality if you believe it makes sense for eGit.
Comment 5 Matthias Sohn CLA 2016-12-15 02:48:46 EST
(In reply to Jaxsun McCarthy Huggan from comment #4)
> Hi Matthias, this was initially reported to Mylyn, hence the description
> about the review editor.
> 
> Mylyn reviews uses the FetchGerritChangeWizard to checkout patch sets from
> gerrit reviews. If this wizard is invoked twice the second time it will open
> and tell the user that they already have a branch with the given refname. It
> would be nice if when a user hits this condition the wizard were to show a
> page prompting them to change to that branch.

ok, understood
 
> I'm sure there are ways to reproduce this with just egit, but I will give
> some simple steps to reproduce via Mylyn since that is how I am familiar
> with this wizard.
> 
> Steps to reproduce:
> * Open a gerrit review with Mylyn
> * choose a patch set and click "Fetch..."
> * complete the fetch wizard
> * change to some other branch (eg. master)
> * open the same review and click "Fetch..." on the same patch set
> * the fetch wizard opens and warns the user that there is already a branch
> with the given name
> 
> I would be interested in contributing this functionality if you believe it
> makes sense for eGit.

yes, this would be useful. I think we should check if the local branch still points
at the same commit you would download using the wizard. The user may for example have rebased the branch locally. Maybe it would make sense to warn the user if the commit is not the original one.

I'd appreciate your contribution, please follow the contributor guide [1].

[1] https://wiki.eclipse.org/EGit/Contributor_Guide#Contributing_Patches
Comment 6 Eclipse Genie CLA 2016-12-18 02:55:34 EST
New Gerrit change created: https://git.eclipse.org/r/87372
Comment 7 Eclipse Genie CLA 2017-03-06 17:56:27 EST
Gerrit change https://git.eclipse.org/r/87372 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=4c6fe7070d5802a7c189a360d0cfbaeb565e20b1
Comment 8 Andrey Loskutov CLA 2017-03-15 17:42:59 EDT
"Fetch from gerrit" doesn't do anything useful anymore as soon as the "checkout new branch" is selected. This is a blocker => reopening.
Comment 9 Matthias Sohn CLA 2017-03-15 18:11:29 EDT
(In reply to Andrey Loskutov from comment #8)
> "Fetch from gerrit" doesn't do anything useful anymore as soon as the
> "checkout new branch" is selected. This is a blocker => reopening.

if we don't get a fix quickly we should revert this change until a fixed version is available
Comment 10 Andrey Loskutov CLA 2017-03-15 18:32:09 EDT
(In reply to Matthias Sohn from comment #9)
> (In reply to Andrey Loskutov from comment #8)
> > "Fetch from gerrit" doesn't do anything useful anymore as soon as the
> > "checkout new branch" is selected. This is a blocker => reopening.
> 
> if we don't get a fix quickly we should revert this change until a fixed
> version is available

I've tried to fix it but I can't debug right now because something is broken in my M6 workspace - I can't use context menus and get only StackOverflows... So please revert, because a simple gerrit patch checkout is not possible anymore.
Comment 11 Eclipse Genie CLA 2017-03-16 05:35:07 EDT
New Gerrit change created: https://git.eclipse.org/r/93183
Comment 12 Andrey Loskutov CLA 2017-03-16 05:36:50 EDT
(In reply to Eclipse Genie from comment #11)
> New Gerrit change created: https://git.eclipse.org/r/93183

I'm reverting the change. I've tried to fix it but the whole handling of those connected radio buttons is broken with this, and currently I can't fetch anymore.
Comment 13 Eclipse Genie CLA 2017-03-16 06:21:15 EDT
Gerrit change https://git.eclipse.org/r/93183 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=49c363c7d6cfe107cfada512ee8e6efcb98ea2b6