RE: [PATCH] git-gui: Modify push dialog to support Gerrit review

2013-09-05 Thread Jørgen Edelbo
Hi,

I am not quite sure what your concern is.

Anyway - I don't think you are any more restricted with this change than 
before. If you currently work on a topic branch, and the same branch exists on 
the chosen remote, then this branch will be the default one to push to. 

BR Jørgen Edelbo

-Original Message-
From: Johannes Sixt [mailto:j.s...@viscovery.net] 
Sent: 5. september 2013 08:42
To: Jørgen Edelbo
Cc: git@vger.kernel.org; spea...@spearce.org; hvo...@hvoigt.net
Subject: 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

2013-09-05 Thread Jørgen Edelbo
> -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

2013-09-06 Thread Jørgen Edelbo
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 {}}{
+   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 
foreach h [load_all_heads] {
$w.source.l insert end $h
if {$h eq $current_branch} {

The idea is to insert a "" 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

2013-09-07 Thread Jørgen Edelbo

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 ". 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