From: Pete Wyckoff <pw@padd.com>
Date: Thu, 20 Oct 2011 01:16:10 GMT
Subject: Re: git-p4.skipSubmitEdit
Message-ID: <20111020011610.GA7292@arf.padd.com>
In-Reply-To: <4E9DBD0B.7020505@diamand.org>

luke@diamand.org wrote on Tue, 18 Oct 2011 18:53 +0100:
> Looks good, one minor nit (see below) and a comment.
[..]
> >+        # invoke the editor
> >+        if os.environ.has_key("P4EDITOR"):
> >+            editor = os.environ.get("P4EDITOR")
> >+        else:
> >+            editor = read_pipe("git var GIT_EDITOR").strip()
> >+        system(editor + " " + template_file)
> 
> This is where we should really check the return code. However, doing
> so seems to break lots of the existing tests so it's not as easy as
> it looks.

Indeed.  I'll not fix that now, but agree it should be.

> >+
> >+        # If the file was not saved, prompt to see if this patch should
> >+        # be skipped.  But skip this verification step if configured so.
> >+        if gitConfig("git-p4.skipSubmitEditCheck") == "true":
> >+            print "return true for skipSubmitEditCheck"
> 
> You print a helpful/annoying(?) message here, but not further up at
> skipSubmitEdit?

Aargh.  Leaked debug code.  Thanks for noticing.  I got rid of
it.

		-- Pete