luke@xxxxxxxxxxx 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html