Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.

From: Andrei Warkentin <awarkentin@vmware.com>
Date: Oct 17, 2011, 16:18 UTC
Message-ID: <83923897.7841.1318868319131.JavaMail.root@zimbra-prod-mbox-2.vmware.com>
In-Reply-To: <4E99E8D2.6020107@diamand.org>

Hi,

----- Original Message -----

Show 41 quoted lines

> From: "Luke Diamand" <luke@diamand.org>
> To: "Andrei Warkentin" <andreiw@vmware.com>
> Cc: git@vger.kernel.org, gitster@pobox.com, "Pete Wyckoff" <pw@padd.com>
> Sent: Saturday, October 15, 2011 4:10:58 PM
> Subject: Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.
> 
> On 14/10/11 22:51, Andrei Warkentin wrote:
> > Many users of p4/sd use changelists for review, regression
> > tests and batch builds, thus changes are almost never directly
> > submitted.
> >
> > This new config option lets a 'p4 change -i' run instead of
> > the 'p4 submit -i'.
> >
> > Signed-off-by: Andrei Warkentin<andreiw@vmware.com>
> > ---
> >   contrib/fast-import/git-p4     |   16 ++++++++++++----
> >   contrib/fast-import/git-p4.txt |   10 ++++++++++
> >   2 files changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/contrib/fast-import/git-p4
> > b/contrib/fast-import/git-p4
> > index 2f7b270..19c295b 100755
> > --- a/contrib/fast-import/git-p4
> > +++ b/contrib/fast-import/git-p4
> > @@ -959,7 +959,10 @@ class P4Submit(Command, P4UserMap):
> >                   submitTemplate =
> >                   message[:message.index(separatorLine)]
> >                   if self.isWindows:
> >                       submitTemplate =
> >                       submitTemplate.replace("\r\n", "\n")
> > -                p4_write_pipe("submit -i", submitTemplate)
> > +                if gitConfig("git-p4.changeOnSubmit"):
> > +                    p4_write_pipe("change -i", submitTemplate)
> > +                else:
> > +                    p4_write_pipe("subadasdmit -i",
> > submitTemplate)
> 
> 
> What does "p4 subadasmit" do? That's a new command to me!
>

Ack, that's emabarrasing. How did that get there :-)?

Anyway, the other suggestion I had was to create a new command instead of overriding behaviour of an existing one. Of course, copy-pasting P4Submit into P4Change is silly, so...

How about something like this?

The commands dict maps command name to class and optional dict passed to cmd.run(). That way 'change' can really mean P4Submit with an extra parameter not to submit but to do a changelist instead. The reason why I initially made the config flag was because I didn't want to copy-paste P4Submit into P4Change.

commands = {
    "debug" : [ P4Debug, {} ]
    "submit" : [ P4Submit, { "doChange" : 0 } ]
    "commit" : [ P4Submit, { "doChange" : 0 } ]
    "change" : [ P4Submit, { "doChange" : 1 } ]
    "sync" : [ P4Sync, {} ],
    "rebase" : [ P4Rebase, {} ],
    "clone" : [ P4Clone, {} ],
    "rollback" : [ P4RollBack, {} ],
    "branches" : [ P4Branches, {} ]
}

Thanks for the review, A