{"thread":{"id":"34864","subject":"[PATCH] git-gui: Modify push dialog to support Gerrit review","startedAt":"2013-09-06T10:30:15Z","lastAt":"2013-09-07T17:03:53Z","messageCount":3,"participants":["Joergen Edelbo","Phil Hord","Jørgen Edelbo"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"226929","messageId":"20130906105022.0718C12156D@jed-dev-01.labnet","threadId":"34864","inReplyTo":null,"subject":"[PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Joergen Edelbo","fromEmail":"jed@napatech.com","sentAt":"2013-09-06T10:30:15Z","receivedAt":"2013-09-06T10:30:15Z","isPatch":true,"sender":{"key":"jed@napatech.com","avatar":null},"body":"Problem: It is not possible to push for Gerrit review as you will\nalways try to push to \"refs/heads/...\" on the remote.\n\nChanges done:\n\nAdd an option to select \"Gerrit review\" and a corresponding entry\nfor a branch name. If this option is selected, push the changes to\n\"refs/for//\". In this way the local branch\nnames will be used as topic branches on Gerrit.\n---\nHi all,\n\nThis is a second attempt to support Gerrit review. It is fully backwards\ncompatible as the Gerrit option is an addition only. It is also better\nthan the first approach as it supports pushing more local branches to\nGerrit - each having their own topic branch there.\n\nFurther improvement could be to make the Gerrit branch specification a\ndrop down list, but I would like to have this first simple approach \nevaluated first.\n\nBR Joergen\n\n lib/transport.tcl | 30 +++++++++++++++++++++++++++++-\n 1 file changed, 29 insertions(+), 1 deletion(-)\n\ndiff --git a/lib/transport.tcl b/lib/transport.tcl\nindex e5d211e..adf2bbb 100644\n--- a/lib/transport.tcl\n+++ b/lib/transport.tcl\n@@ -61,6 +61,7 @@ proc push_to {remote} {\n \n proc start_push_anywhere_action {w} {\n \tglobal push_urltype push_remote push_url push_thin push_tags\n+\tglobal gerrit_review gerrit_branch\n \tglobal push_force\n \tglobal repo_config\n \n@@ -95,7 +96,15 @@ proc start_push_anywhere_action {w} {\n \t\tset cnt 0\n \t\tforeach i [$w.source.l curselection] {\n \t\t\tset b [$w.source.l get $i]\n-\t\t\tlappend cmd \"refs/heads/$b:refs/heads/$b\"\n+\t\t\tif {$gerrit_review && $gerrit_branch ne {}} {\n+\t\t\t\tif {$gerrit_branch eq $b} {\n+\t\t\t\t\tlappend cmd \"refs/heads/$b:refs/for/$b\"\n+\t\t\t\t} else {\n+\t\t\t\t\tlappend cmd \"refs/heads/$b:refs/for/$gerrit_branch/$b\"\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\tlappend cmd \"refs/heads/$b:refs/heads/$b\"\n+\t\t\t}\n \t\t\tincr cnt\n \t\t}\n \t\tif {$cnt == 0} {\n@@ -120,6 +129,7 @@ trace add variable push_remote write \\\n proc do_push_anywhere {} {\n \tglobal all_remotes current_branch\n \tglobal push_urltype push_remote push_url push_thin push_tags\n+\tglobal gerrit_review gerrit_branch\n \tglobal push_force use_ttk NS\n \n \tset w .push_setup\n@@ -215,6 +225,24 @@ proc do_push_anywhere {} {\n \t\t-text [mc \"Include tags\"] \\\n \t\t-variable push_tags\n \tgrid $w.options.tags -columnspan 2 -sticky w\n+\t${NS}::checkbutton $w.options.gerrit \\\n+\t\t-text [mc \"Gerrit review. Branch: \"] \\\n+\t\t-variable gerrit_review\n+\t${NS}::entry $w.options.gerrit_br \\\n+\t\t-width 50 \\\n+\t\t-textvariable gerrit_branch \\\n+\t\t-validate key \\\n+\t\t-validatecommand {\n+\t\t\tif {%d == 1 && [regexp {\\s} %S]} {return 0}\n+\t\t\tif {%d == 1 && [string length %S] > 0} {\n+\t\t\t\tset gerrit_review 1\n+\t\t\t}\n+\t\t\tif {[string length %P] == 0} {\n+\t\t\t\tset gerrit_review 0\n+\t\t\t}\n+\t\t\treturn 1\n+\t\t}\n+\tgrid $w.options.gerrit $w.options.gerrit_br -sticky we -padx {0 5}\n \tgrid columnconfigure $w.options 1 -weight 1\n \tpack $w.options -anchor nw -fill x -pady 5 -padx 5\n \n-- \n1.7.9.5\n"},{"id":"226992","messageId":"CABURp0qL6CSmt87OnTpRjS4VFikfA3O+w7-HgfB9d6Kip46EpA@mail.gmail.com","threadId":"34864","inReplyTo":"20130906105022.0718C12156D@jed-dev-01.labnet","subject":"Re: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-09-06T21:49:51Z","receivedAt":"2013-09-06T21:49:51Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Fri, Sep 6, 2013 at 6:30 AM, Joergen Edelbo wrote:\n> Problem: It is not possible to push for Gerrit review as you will\n> always try to push to \"refs/heads/...\" on the remote.\n>\n> Changes done:\n>\n> Add an option to select \"Gerrit review\" and a corresponding entry\n> for a branch name. If this option is selected, push the changes to\n> \"refs/for//\". In this way the local branch\n> names will be used as topic branches on Gerrit.\n\n\nIn my gerrit repos, I have this configuration\n\n $ git config remote.origin.push HEAD:refs/for/master\n\nAnd so I can simply 'git push' and git does what I mean.\n\nMy main complaint with git-gui's push is that it ignores my\nconfiguration. Can you teach git-gui to honor this setting, instead?\n\nI would like for this remote.\"name\".push option to be smarter and figure\nout which $branch I mean when I am not on master, but that is a different\ndiscussion. Having said that, I see that your change to git-gui attempts to\nmake that automatic. Kudos for that, but I don't think this will work for\nme as I am often on a detached branch and so \"refs/heads/$b\" is meaningless.\n\nCan you think of a sane way to separate the \"from\" and the \"to\" branches in\nthe GUI? I mean, I would like to push \"HEAD\" and I would like it to\ngo to \"refs/for/frotz-2.0\".\n\nPhil\n"},{"id":"227026","messageId":"522B5C79.3000107@napatech.com","threadId":"34864","inReplyTo":"CABURp0qL6CSmt87OnTpRjS4VFikfA3O+w7-HgfB9d6Kip46EpA@mail.gmail.com","subject":"Re: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Jørgen Edelbo","fromEmail":"jed@napatech.com","sentAt":"2013-09-07T17:03:53Z","receivedAt":"2013-09-07T17:03:53Z","isPatch":true,"sender":{"key":"jed@napatech.com","avatar":null},"body":"On 06-09-2013 23:49, Phil Hord wrote:\n> Can you think of a sane way to separate the \"from\" and the \"to\" branches in\n> the GUI? I mean, I would like to push \"HEAD\" and I would like it to\n> go to \"refs/for/frotz-2.0\".\n\nMy first attemt at this change was to do do exactly that: always push \nHEAD, and being able to specify the destination branches. If it was a \nGerrit review server, it would replace refs/heads/... with refs/for/...\n\nHowever, it is now clear, that we have to support specifying the \nbranches to push.\n\nMy next thought was: could we add an entry to the list of branches to \npush, meaning \"omit the branch specs in the command\". Then it should \njust perform a \"git push \". What was then pushed would depend\non the push configuration for the remote. In the Gerrit case we could \nthen have:\n push = HEAD:refs/for/master, or whatever.\n\nThe drawback of this solution is that the UI is not so intuitive and it \nwould not support the case where you would want to push your branch \nfrotz to refs/for/master/frotz, which is the way you specify a topic in \nGerrit. The current solution supports this.\n\nWhat remains to support is the case where you work on a detached HEAD. \nMost of it should be straight forward. The gui knows we are in detached \nstate, so in this case, a \"HEAD\" entry could be added to the list of \nbranches. The question is just: to which branch to push to in the non \nGerrit case? Perhaps the ref specs could just be left out in this case.\n"}]}