Re: [PATCH] git-gui: Modify push dialog to support Gerrit review
On 06-09-2013 23:49, Phil Hord wrote: Can you think of a sane way to separate the from and the to branches in the GUI? I mean, I would like to push HEAD and I would like it to go to refs/for/frotz-2.0. My first attemt at this change was to do do exactly that: always push HEAD, and being able to specify the destination branches. If it was a Gerrit review server, it would replace refs/heads/... with refs/for/... However, it is now clear, that we have to support specifying the branches to push. My next thought was: could we add an entry to the list of branches to push, meaning omit the branch specs in the command. Then it should just perform a git push remote. What was then pushed would depend on the push configuration for the remote. In the Gerrit case we could then have: push = HEAD:refs/for/master, or whatever. The drawback of this solution is that the UI is not so intuitive and it would not support the case where you would want to push your branch frotz to refs/for/master/frotz, which is the way you specify a topic in Gerrit. The current solution supports this. What remains to support is the case where you work on a detached HEAD. Most of it should be straight forward. The gui knows we are in detached state, so in this case, a HEAD entry could be added to the list of branches. The question is just: to which branch to push to in the non Gerrit case? Perhaps the ref specs could just be left out in this case. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] git-gui: Modify push dialog to support Gerrit review
Junio C Hamano gits...@pobox.com writes:
Isn't the right way to improve the situation to let the command line tools
know how the user wants to push things out and just have Git-GUI delegate
the choice to the underlying git push?
Thank you for all the constructive feedback. I realize that it is not the way
forward to remove the selection of branches to push.
What I consider now, is to pursue the idea that Junio presents above: just let
the gui tool use whatever is configured for the selected remote. I have,
however not been able to come up with a solution that looks nice. What I have
been trying now is the following little modification:
diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
index e5d211e..1709464 100644
--- a/git-gui/lib/transport.tcl
+++ b/git-gui/lib/transport.tcl
@@ -95,7 +95,9 @@ proc start_push_anywhere_action {w} {
set cnt 0
foreach i [$w.source.l curselection] {
set b [$w.source.l get $i]
- lappend cmd refs/heads/$b:refs/heads/$b
+ if {$b nw {default}}{
+ lappend cmd refs/heads/$b:refs/heads/$b
+ }
incr cnt
}
if {$cnt == 0} {
@@ -149,6 +151,7 @@ proc do_push_anywhere {} {
-height 10 \
-width 70 \
-selectmode extended
+ $w.source.l insert end default
foreach h [load_all_heads] {
$w.source.l insert end $h
if {$h eq $current_branch} {
The idea is to insert a default entry in the branch selection list, and
then skip that when building the command. Then you can avoid having refs on the
command line and just let git act according to configuration.
How about that? Any smarter way to do it?
BR Jørgen
Re: [PATCH] git-gui: Modify push dialog to support Gerrit review
On Fri, Sep 6, 2013 at 6:30 AM, Joergen Edelbo j...@napatech.com wrote: Problem: It is not possible to push for Gerrit review as you will always try to push to refs/heads/... on the remote. Changes done: Add an option to select Gerrit review and a corresponding entry for a branch name. If this option is selected, push the changes to refs/for/gerrit-branch/local branch. In this way the local branch names will be used as topic branches on Gerrit. In my gerrit repos, I have this configuration $ git config remote.origin.push HEAD:refs/for/master And so I can simply 'git push' and git does what I mean. My main complaint with git-gui's push is that it ignores my configuration. Can you teach git-gui to honor this setting, instead? I would like for this remote.name.push option to be smarter and figure out which $branch I mean when I am not on master, but that is a different discussion. Having said that, I see that your change to git-gui attempts to make that automatic. Kudos for that, but I don't think this will work for me as I am often on a detached branch and so refs/heads/$b is meaningless. Can you think of a sane way to separate the from and the to branches in the GUI? I mean, I would like to push HEAD and I would like it to go to refs/for/frotz-2.0. Phil -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-gui: Modify push dialog to support Gerrit review
Am 9/2/2013 10:54, schrieb Joergen Edelbo: Changes done: Remove selection of branches to push - push always HEAD. This can be justified by the fact that this far the most common thing to do. What are your plans to support a topic-based workflow? Far the most common thing to happen is that someone forgets to push completed topics. With this change, aren't those people forced to relinguish their current work because they have to checkout the completed topics to push them? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] git-gui: Modify push dialog to support Gerrit review
-Original Message- From: Johannes Sixt [mailto:j.s...@viscovery.net] Sent: 5. september 2013 10:57 Please do not top-post. Am 9/5/2013 10:29, schrieb Jørgen Edelbo: -Original Message- From: Johannes Sixt Am 9/2/2013 10:54, schrieb Joergen Edelbo: Changes done: Remove selection of branches to push - push always HEAD. This can be justified by the fact that this far the most common thing to do. What are your plans to support a topic-based workflow? Far the most common thing to happen is that someone forgets to push completed topics. With this change, aren't those people forced to relinguish their current work because they have to checkout the completed topics to push them? I am not quite sure what your concern is. When I have completed topics A and B, but forgot to push them, and now I am working on topic C, how do I push topics A and B? You say I can only push HEAD. I understand this that I have to stop work on C (perhaps commit or stash any unfinished work), then checkout A, push it, checkout B, push it, checkout C and unstash the unfinished work. If my understanding is correct, the new restriction is a no-go. Ok, this way of working is not supported. It just never occurred to me that you would work this way. Forgetting to push something that you have just completed is very far from what I am used to. I think it comes most natural to push what you have done before changing topic. The reason I make a commit is to get it out of the door. -- Hannes - Jørgen -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-gui: Modify push dialog to support Gerrit review
Am 9/5/2013 11:18, schrieb Jørgen Edelbo: Forgetting to push something that you have just completed is very far from what I am used to. Forgetting to push is just one of many reasons why a branch that is not equal to HEAD is not yet pushed... The new restriction is just too tight. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-gui: Modify push dialog to support Gerrit review
Jørgen Edelbo j...@napatech.com writes:
You say I can only push HEAD. I understand this that I have to stop work on C
(perhaps commit or stash any unfinished work), then checkout A, push it,
checkout B, push it, checkout C and unstash the unfinished work. If my
understanding is correct, the new restriction is a no-go.
Ok, this way of working is not supported. It just never occurred to me that
you would work this way. Forgetting to push something that you have just
completed is very far from what I am used to. I think it comes most natural
to push what you have done before changing topic. The reason I make a commit
is to get it out of the door.
People work in very different ways, and mine and yours are extreme
opposites. I almost never push out a commit that is just off the
press, I use topic branches heavily and work on multiple topics
(either related or unrelated) in parallel all the time, so it is
very usual for me to push out more than one branches with a single
push---by definition, if we support pushing only the current branch
out, working on more than one topics and after perfecting these,
push them together cannot be done.
If one sets push.default to something other than the matching, and
does not have any remote.*.push refspec in the configuration, J6t's
I forgot to push while I was on that branch and my I deliberately
did not push those branches out while I was on them, but now I know
I am ready to push them out cannot be done without explicit
refspecs on the command line.
But stepping back a bit, I think this I commit because I want to
get it out the door is coming from the same desire that led to a
possible design mistake that made various push.default settings push
only the current branch out.
The possible design mistake that has been disturbing me is the
following.
The push.default setting controls two unrelated things, and that
is one too many. Between the matching modes and the rest, it tells
what branch(es) are pushed out (either all the branches with the
same name or the current branch). That is one thing, and I tend
to agree that push only the current branch would be a sane default
many people would prefer (and that is the reason we made it the
default for Git 2.0).
Among the various non matching modes, it also determines where a
branch that is pushed out would go (current pushes to the same
name, upstream pushes to @{upstream}, etc.). But this once I
choose what branch to push out, the branch being pushed out knows
where it wants to go, does not take effect if one explicitly names
what to push, e.g. in this sequence where one forgets to push
'frotz' out:
j6t$ git checkout frotz
... work work work ...
j6t$ git checkout xyzzy
... work work work ...
... realize 'frotz' is done
j6t$ git push origin frotz
The last push may work differently from the push in this sequence:
j6t$ git checkout frotz
... work work work ...
j6t$ git push ;# or git push origin
In the latter sequence, the niceties of push.upstream to push
'frotz' out to the frotz@{upstream} (and your git push origin
refs/heads/frotz:refs/for/frotz mapping done in git-gui) will take
effect, but in the former, the refspec frotz on the command line
is taken as a short-hand for frotz:frotz.
We may want to teach git push that the command line refspec that
names a local branch may not be pushing to the same name but wants
to go through the same mapping used when git push is done while
the branch is checked out, with some mechanism.
It is a separate but very related topic, I think.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-gui: Modify push dialog to support Gerrit review
Joergen Edelbo j...@napatech.com writes:
+proc get_remote_branch {} {
+ global push_branchtype push_branch push_new
+ set branch {}
+ switch -- $push_branchtype {
+ existing { set branch $push_branch }
+ create { set branch $push_new }
+ }
+ return $branch
+}
+
+proc get_remote_ref_spec {} {
+ global gerrit_review
+ set push_branch [get_remote_branch]
+ if {$gerrit_review} {
+ return refs/for/$push_branch
+ } else {
+ return refs/heads/$push_branch
}
+}
I am puzzled. This may be fine for those who use Git-GUI and
nothing else to push, but will not help whose who use both Git-GUI
and the command line.
Isn't the right way to improve the situation to let the command line
tools know how the user wants to push things out and just have
Git-GUI delegate the choice to the underlying git push?
For example, if you are working on a topic 'frotz', and if the
location you push is managed by Gerrit, isn't it the case that you
always want to push it to refs/for/frotz, whether you are pushing
via Git-GUI or from the command line?
I think we discussed during 1.8.4 cycle a configuration like this:
[branch frotz]
push = refs/heads/frotz:refs/for/frotz
as part of the triangular workflow topic that allows you to
specify that when 'frotz' is pushed out, it goes to
refs/for/frotz, or something like that, but I do not recall what
came out of that.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RE: [PATCH] git-gui: Modify push dialog to support Gerrit review
On Thu, Sep 05, 2013 at 09:18:25AM +, Jørgen Edelbo wrote: -Original Message- From: Johannes Sixt [mailto:j.s...@viscovery.net] Sent: 5. september 2013 10:57 Please do not top-post. Am 9/5/2013 10:29, schrieb Jørgen Edelbo: -Original Message- From: Johannes Sixt Am 9/2/2013 10:54, schrieb Joergen Edelbo: Changes done: Remove selection of branches to push - push always HEAD. This can be justified by the fact that this far the most common thing to do. What are your plans to support a topic-based workflow? Far the most common thing to happen is that someone forgets to push completed topics. With this change, aren't those people forced to relinguish their current work because they have to checkout the completed topics to push them? I am not quite sure what your concern is. When I have completed topics A and B, but forgot to push them, and now I am working on topic C, how do I push topics A and B? You say I can only push HEAD. I understand this that I have to stop work on C (perhaps commit or stash any unfinished work), then checkout A, push it, checkout B, push it, checkout C and unstash the unfinished work. If my understanding is correct, the new restriction is a no-go. Ok, this way of working is not supported. It just never occurred to me that you would work this way. Forgetting to push something that you have just completed is very far from what I am used to. I think it comes most natural to push what you have done before changing topic. The reason I make a commit is to get it out of the door. FWIW, I also think that we should keep the box which allows you to select the branch to push. I did not realize that you were removing it when I first glanced at your patch. Even if your reasoning that pushing the currently checked out branch is correct: This box has been around for too long, so it will annoy people that got used to the fact that they can select the branch to push. Another problem: It is not very intuitive to only select the branch to push to. You can do that on the command line but IMO using git push origin HEAD:refs/heads/branchname is way less common than git push origin branchname and I think that should also be reflected in the gui. It might be more common for a gerrit user but for the typical git user without gerrit it is not. So to make it easy for the user to transition from gui to commandline and back with your patch I would expect: The user selects a branch to push. The new Destination Branches section automatically selects/shows the same name for the default case as destination (like the cli). So if I only select the branch to push it behaves the same as before. If you detect (I assume that is possible somehow) that the remote is a gerrit remote: Push for Gerrit review would automatically be ticked and the branch a git pull would merge (e.g. the one from branch.name.merge) is selected as the destination branch under refs/for/... . If there is no config for that, fallback to master. This is what I would expect with no further extension of the current git command line behavior and config options. So that way your patch will be an *extension* and not a change of behavior. Another unrelated thing that is currently left out: You can transport the local branchname when pushing to the magical gerrit refs/for/... . I would like to see that appended as well. But opposed to the branch selection that is not a show stopper for the patch more a side note. Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
