Re: git-p4.skipSubmitEdit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I agree with almost all your points. I have answered them each in-line
below.

On Mon, 2011-09-12 at 03:34 -0400, Luke Diamand wrote:
> On 08/09/11 21:40, L. A. Linden Levy wrote:
> > Hi All,
> >
> > I have been using git-p4 for a while and it has allowed me to completely
> > change the way I develop and still be able to use perforce which my
> > company has for its main VCS. One thing that was driving me nuts was
> > that "git p4 submit" cycles through all of my individual commits and
> > asks me if I want to change them. The way I develop I often am checking
> > in 20 to 50 different small commits each with a descriptive git comment.
> > I felt like I was doing double duty by having emacs open on every commit
> > into perforce. So I modified git-p4 to have an option to skip the
> > editor. This option coupled with git-p4.skipSubmitEditCheck will make
> > the submission non-interactive for "git p4 submit".
> 
> 
> Sorry - I've not had a chance to look at this before now. But a couple 
> of comments:
> 
>   - Is there a line wrap problem in the patch? It doesn't seem to want 
> to apply for me.

Probably. Below are the results from following the patch submission
instructions. 

From 16c4344de0047cbaf3381eca590a3e59b0d0a25c Mon Sep 17 00:00:00 2001
From: "Loren A. Linden Levy" <lindenle@xxxxxxxxx>
Date: Thu, 8 Sep 2011 13:37:22 -0700
Subject: [PATCH 1/5] changed git-p4

---
 contrib/fast-import/git-p4 |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2f7b270..a438b3e 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -935,18 +935,23 @@ class P4Submit(Command, P4UserMap):
             tmpFile.write(submitTemplate + separatorLine + diff +
newdiff)
             tmpFile.close()
             mtime = os.stat(fileName).st_mtime
-            if os.environ.has_key("P4EDITOR"):
-                editor = os.environ.get("P4EDITOR")
+            if gitConfig("git-p4.skipSubmitEdit") == "true":
+                pass
             else:
-                editor = read_pipe("git var GIT_EDITOR").strip()
-            system(editor + " " + fileName)
-
+                if os.environ.has_key("P4EDITOR"):
+                    editor = os.environ.get("P4EDITOR")
+                else:
+                    editor = read_pipe("git var GIT_EDITOR").strip()
+                    
+                    system(editor + " " + fileName)
+                    
             if gitConfig("git-p4.skipSubmitEditCheck") == "true":
                 checkModTime = False
             else:
                 checkModTime = True
 
             response = "y"
+
             if checkModTime and (os.stat(fileName).st_mtime <= mtime):
                 response = "x"
                 while response != "y" and response != "n":
-- 
1.7.7.rc0.73.g16c43

From ed60bb737b89eea4b84719c458aeebebd666a2ae Mon Sep 17 00:00:00 2001
From: "Loren A. Linden Levy" <lindenle@xxxxxxxxx>
Date: Mon, 12 Sep 2011 09:52:52 -0700
Subject: [PATCH 2/5] removed white space for patch

---
 contrib/fast-import/git-p4 |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index a438b3e..edd2525 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -942,9 +942,7 @@ class P4Submit(Command, P4UserMap):
                     editor = os.environ.get("P4EDITOR")
                 else:
                     editor = read_pipe("git var GIT_EDITOR").strip()
-                    
                     system(editor + " " + fileName)
-                    
             if gitConfig("git-p4.skipSubmitEditCheck") == "true":
                 checkModTime = False
             else:
-- 
1.7.7.rc0.73.g16c43

From ee7eaf73cf52100359b08600eab1279e03ef777b Mon Sep 17 00:00:00 2001
From: "Loren A. Linden Levy" <lindenle@xxxxxxxxx>
Date: Mon, 12 Sep 2011 09:55:11 -0700
Subject: [PATCH 3/5] modified the documentation

---
 contrib/fast-import/git-p4.txt |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4.txt
b/contrib/fast-import/git-p4.txt
index 52003ae..3dba636 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -202,10 +202,16 @@ able to find the relevant client.  This client
spec will be used to
 both filter the files cloned by git and set the directory layout as
 specified in the client (this implies --keep-path style semantics).
 
-git-p4.skipSubmitModTimeCheck
+git-p4.skipSubmitEdit
 
   git config [--global] git-p4.skipSubmitModTimeCheck false
 
+If true, submit will not invoke the editor to modify the p4 change
template.
+
+git-p4.skipSubmitEditCheck 
+
+  git config [--global] git-p4.skipSubmitEditCheck false
+
 If true, submit will not check if the p4 change template has been
modified.
 
 git-p4.preserveUser
-- 
1.7.7.rc0.73.g16c43

From ee7eaf73cf52100359b08600eab1279e03ef777b Mon Sep 17 00:00:00 2001
From: "Loren A. Linden Levy" <lindenle@xxxxxxxxx>
Date: Mon, 12 Sep 2011 09:55:11 -0700
Subject: [PATCH 3/6] modified the documentation

---
 contrib/fast-import/git-p4.txt |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4.txt
b/contrib/fast-import/git-p4.txt
index 52003ae..3dba636 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -202,10 +202,16 @@ able to find the relevant client.  This client
spec will be used to
 both filter the files cloned by git and set the directory layout as
 specified in the client (this implies --keep-path style semantics).
 
-git-p4.skipSubmitModTimeCheck
+git-p4.skipSubmitEdit
 
   git config [--global] git-p4.skipSubmitModTimeCheck false
 
+If true, submit will not invoke the editor to modify the p4 change
template.
+
+git-p4.skipSubmitEditCheck 
+
+  git config [--global] git-p4.skipSubmitEditCheck false
+
 If true, submit will not check if the p4 change template has been
modified.
 
 git-p4.preserveUser
-- 
1.7.7.rc0.73.g16c43

From c9d3b4ea4dcf1f5fd489dfacde550dd4c648afc7 Mon Sep 17 00:00:00 2001
From: "Loren A. Linden Levy" <lindenle@xxxxxxxxx>
Date: Mon, 12 Sep 2011 10:01:08 -0700
Subject: [PATCH 4/6] changed the order a bit

---
 contrib/fast-import/git-p4 |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index edd2525..80bcc90 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -935,18 +935,19 @@ class P4Submit(Command, P4UserMap):
             tmpFile.write(submitTemplate + separatorLine + diff +
newdiff)
             tmpFile.close()
             mtime = os.stat(fileName).st_mtime
+            if gitConfig("git-p4.skipSubmitEditCheck") == "true":
+                checkModTime = False
+            else:
+                checkModTime = True
             if gitConfig("git-p4.skipSubmitEdit") == "true":
-                pass
+                checkModTime = False
+                pass        
             else:
                 if os.environ.has_key("P4EDITOR"):
                     editor = os.environ.get("P4EDITOR")
                 else:
                     editor = read_pipe("git var GIT_EDITOR").strip()
                     system(editor + " " + fileName)
-            if gitConfig("git-p4.skipSubmitEditCheck") == "true":
-                checkModTime = False
-            else:
-                checkModTime = True
 
             response = "y"
 
-- 
1.7.7.rc0.73.g16c43

From de3d67a301b49e4d986e83727b822aa5fd257527 Mon Sep 17 00:00:00 2001
From: "Loren A. Linden Levy" <lindenle@xxxxxxxxx>
Date: Mon, 12 Sep 2011 10:03:03 -0700
Subject: [PATCH 5/6] cleaned up whitespace for making a patch

---
 contrib/fast-import/git-p4     |    2 +-
 contrib/fast-import/git-p4.txt |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 80bcc90..1cd65a3 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -941,7 +941,7 @@ class P4Submit(Command, P4UserMap):
                 checkModTime = True
             if gitConfig("git-p4.skipSubmitEdit") == "true":
                 checkModTime = False
-                pass        
+                pass
             else:
                 if os.environ.has_key("P4EDITOR"):
                     editor = os.environ.get("P4EDITOR")
diff --git a/contrib/fast-import/git-p4.txt
b/contrib/fast-import/git-p4.txt
index 3dba636..8291e7a 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -208,7 +208,7 @@ git-p4.skipSubmitEdit
 
 If true, submit will not invoke the editor to modify the p4 change
template.
 
-git-p4.skipSubmitEditCheck 
+git-p4.skipSubmitEditCheck
 
   git config [--global] git-p4.skipSubmitEditCheck false
 
-- 
1.7.7.rc0.73.g16c43

From 64c4878d12748daeda637b3d7d2f018c316b4239 Mon Sep 17 00:00:00 2001
From: "Loren A. Linden Levy" <lindenle@xxxxxxxxx>
Date: Mon, 12 Sep 2011 10:10:22 -0700
Subject: [PATCH 6/6] made the docs correct for the new option

---
 contrib/fast-import/git-p4.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4.txt
b/contrib/fast-import/git-p4.txt
index 8291e7a..f1c1d0c 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -204,7 +204,7 @@ specified in the client (this implies --keep-path
style semantics).
 
 git-p4.skipSubmitEdit
 
-  git config [--global] git-p4.skipSubmitModTimeCheck false
+  git config [--global] git-p4.skipSubmitEdit false
 
 If true, submit will not invoke the editor to modify the p4 change
template.
 
-- 
1.7.7.rc0.73.g16c43



>   - needs an update to the documentation in git-p4.txt

Done. Also I think the skipSubmitEditCheck was incorrectly name
git-p4.skipSubmitModTimeCheck in the git-p4.txt file. I updated this as
well.

>   - is there any way to eliminate the slightly ugly 
> "if/pass/else/do-stuff" construct?

I think it is logically correct and describes the situation well. We are
skipping the editor so we pass it. If you still do not agree then
perhaps a suggestion of a better logical flow would help me. 

>   - I'd think if you turned off the editor completely then there's no 
> point doing the submit-edit-check.

I have changed the ordering so that at the moment this happens because I
agree with you.

>   - You probably need to follow the instructions in 
> Documentation/SubmittingPatches so that Junio will pay attention to the 
> patch.

See point #1.

> Regards!
> Luke
> 
> 
> >
> > Below are the patch and environment results:
> >
> >
> > $ git config -l
> > ...
> > user.name=Loren A. Linden Levy
> > git-p4.skipsubmitedit=true
> > git-p4.skipsubmiteditcheck=true
> > ...
> >
> > $ git format-patch origin/master --stdout
> >
> >  From 16c4344de0047cbaf3381eca590a3e59b0d0a25c Mon Sep 17 00:00:00 2001
> > From: "Loren A. Linden Levy"<lindenle@xxxxxxxxx>
> > Date: Thu, 8 Sep 2011 13:37:22 -0700
> > Subject: [PATCH] changed git-p4
> >
> > ---
> >   contrib/fast-import/git-p4 |   15 ++++++++++-----
> >   1 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> > index 2f7b270..a438b3e 100755
> > --- a/contrib/fast-import/git-p4
> > +++ b/contrib/fast-import/git-p4
> > @@ -935,18 +935,23 @@ class P4Submit(Command, P4UserMap):
> >               tmpFile.write(submitTemplate + separatorLine + diff +
> > newdiff)
> >               tmpFile.close()
> >               mtime = os.stat(fileName).st_mtime
> > -            if os.environ.has_key("P4EDITOR"):
> > -                editor = os.environ.get("P4EDITOR")
> > +            if gitConfig("git-p4.skipSubmitEdit") == "true":
> > +                pass
> >               else:
> > -                editor = read_pipe("git var GIT_EDITOR").strip()
> > -            system(editor + " " + fileName)
> > -
> > +                if os.environ.has_key("P4EDITOR"):
> > +                    editor = os.environ.get("P4EDITOR")
> > +                else:
> > +                    editor = read_pipe("git var GIT_EDITOR").strip()
> > +
> > +                    system(editor + " " + fileName)
> > +
> >               if gitConfig("git-p4.skipSubmitEditCheck") == "true":
> >                   checkModTime = False
> >               else:
> >                   checkModTime = True
> >
> >               response = "y"
> > +
> >               if checkModTime and (os.stat(fileName).st_mtime<= mtime):
> >                   response = "x"
> >                   while response != "y" and response != "n":
> 

-- 
Alex Linden Levy
Senior Software Engineer
MobiTV, Inc.
6425 Christie Avenue, 5th Floor, Emeryville, CA 94608
phone 510.450.5190 mobile 720.352.8394
email alevy@xxxxxxxxxx  web www.mobitv.com

Attachment: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]