[PATCH] Adding list of p4 jobs to git commit message

[PATCH] Adding list of p4 jobs to git commit message

[Jan Durovec]

---
 git-p4.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/git-p4.py b/git-p4.py
index 527d44b..a81795f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2320,6 +2320,15 @@ def extractFilesFromCommit(self, commit):
             fnum = fnum + 1
         return files
 
+    def extractJobsFromCommit(self, commit):
+        jobs = []
+        jnum = 0
+        while commit.has_key("job%s" % jnum):
+            job = commit["job%s" % jnum]
+            jobs.append(job)
+            jnum = jnum + 1
+        return jobs
+
     def stripRepoPath(self, path, prefixes):
         """When streaming files, this is called to map a p4 depot path
            to where it should go in git.  The prefixes are either
@@ -2665,6 +2674,7 @@ def hasBranchPrefix(self, path):
     def commit(self, details, files, branch, parent = ""):
         epoch = details["time"]
         author = details["user"]
+        jobs = self.extractJobsFromCommit(details)
 
         if self.verbose:
             print('commit into {0}'.format(branch))
@@ -2696,6 +2706,8 @@ def commit(self, details, files, branch, parent = ""):
                              (','.join(self.branchPrefixes), details["change"]))
         if len(details['options']) > 0:
             self.gitStream.write(": options = %s" % details['options'])
+        if len(jobs) > 0:
+            self.gitStream.write(": jobs = %s" % (','.join(jobs)))
         self.gitStream.write("]\nEOT\n\n")
 
         if len(parent) > 0:

--
https://github.com/git/git/pull/225

Re: [PATCH] Adding list of p4 jobs to git commit message

[Junio C Hamano]

Jan Durovec <jan.durovec@gmail.com> writes:

> ---

A few issues.  Please:

 (1) Sign-off your work.

 (2) Try to find those who are familiar with the area and Cc them.

     "git shortlog -s -n --since=18.months --no-merges git-p4.py"
     may help.

 (3) Follow the style of existing commits when giving a title to
     your patch.

     "git shortlog --since=18.months --no-merges git-p4.py" may
     help you notice "git-p4: do this thing" is the common way to
     title "git p4" patches.

 (4) Justify why your change is a good thing in your log message.
     What you did, i.e. "list p4 jobs when making a commit", can be
     seen by the patch, but readers cannot guess why you thought it
     is a good idea to extract "job%d" out of the P4 commit and to
     record them in the resulting Git commit, unless you explain
     things like:

     - what goes wrong if you don't?
     - when would "job%d" appear in P4 commit?
     - is it sane to assume "job0", "job1",... appear consecutively?

 (5) Describe what your change does clearly.  "Adding list" is not
     quite clear.  Where in the "git commit message" are you adding
     the list, and why is that location in the message the most
     appropriate place to add it?

Thanks.

>  git-p4.py | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/git-p4.py b/git-p4.py
> index 527d44b..a81795f 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2320,6 +2320,15 @@ def extractFilesFromCommit(self, commit):
>              fnum = fnum + 1
>          return files
>  
> +    def extractJobsFromCommit(self, commit):
> +        jobs = []
> +        jnum = 0
> +        while commit.has_key("job%s" % jnum):
> +            job = commit["job%s" % jnum]
> +            jobs.append(job)
> +            jnum = jnum + 1
> +        return jobs
> +
>      def stripRepoPath(self, path, prefixes):
>          """When streaming files, this is called to map a p4 depot path
>             to where it should go in git.  The prefixes are either
> @@ -2665,6 +2674,7 @@ def hasBranchPrefix(self, path):
>      def commit(self, details, files, branch, parent = ""):
>          epoch = details["time"]
>          author = details["user"]
> +        jobs = self.extractJobsFromCommit(details)
>  
>          if self.verbose:
>              print('commit into {0}'.format(branch))
> @@ -2696,6 +2706,8 @@ def commit(self, details, files, branch, parent = ""):
>                               (','.join(self.branchPrefixes), details["change"]))
>          if len(details['options']) > 0:
>              self.gitStream.write(": options = %s" % details['options'])
> +        if len(jobs) > 0:
> +            self.gitStream.write(": jobs = %s" % (','.join(jobs)))
>          self.gitStream.write("]\nEOT\n\n")
>  
>          if len(parent) > 0:
>
> --
> https://github.com/git/git/pull/225

Re: [PATCH] Adding list of p4 jobs to git commit message

[Luke Diamand]

On 15 April 2016 at 21:27, Junio C Hamano <gitster@pobox.com> wrote:
> Jan Durovec <jan.durovec@gmail.com> writes:
>
>> ---
>
> A few issues.  Please:
>
>  (1) Sign-off your work.
>
>  (2) Try to find those who are familiar with the area and Cc them.
>
>      "git shortlog -s -n --since=18.months --no-merges git-p4.py"
>      may help.
>
>  (3) Follow the style of existing commits when giving a title to
>      your patch.
>
>      "git shortlog --since=18.months --no-merges git-p4.py" may
>      help you notice "git-p4: do this thing" is the common way to
>      title "git p4" patches.
>
>  (4) Justify why your change is a good thing in your log message.
>      What you did, i.e. "list p4 jobs when making a commit", can be
>      seen by the patch, but readers cannot guess why you thought it
>      is a good idea to extract "job%d" out of the P4 commit and to
>      record them in the resulting Git commit, unless you explain
>      things like:
>
>      - what goes wrong if you don't?
>      - when would "job%d" appear in P4 commit?
>      - is it sane to assume "job0", "job1",... appear consecutively?
>
>  (5) Describe what your change does clearly.  "Adding list" is not
>      quite clear.  Where in the "git commit message" are you adding
>      the list, and why is that location in the message the most
>      appropriate place to add it?

Additionally, it would be very useful to add a test case (see t/t98*).
There are quite a few test cases for git-p4, and they make maintenance
a lot easier.

Some documentation (Documentation/git-p4.txt) would also be useful.

Thanks
Luke

Re: [PATCH] Adding list of p4 jobs to git commit message

[Jan Durovec]

Hi,

I tried to address all the issues and (among other things) I've added
a new test case to t/t98 group (t9829-git-p4-jobs.sh).

Unfortunately, Travis CI build now fails with "non-executable tests:
t9829-git-p4-jobs.sh" (https://travis-ci.org/git/git/jobs/123555944)

Can you please advise how to mark it as executable? Does it need to be
added to some configuration file? Or am I interpreting the error
message incorrectly?

Regards,
Jan

On Sat, Apr 16, 2016 at 12:10 AM, Luke Diamand <luke@diamand.org> wrote:
> On 15 April 2016 at 21:27, Junio C Hamano <gitster@pobox.com> wrote:
>> Jan Durovec <jan.durovec@gmail.com> writes:
>>
>>> ---
>>
>> A few issues.  Please:
>>
>>  (1) Sign-off your work.
>>
>>  (2) Try to find those who are familiar with the area and Cc them.
>>
>>      "git shortlog -s -n --since=18.months --no-merges git-p4.py"
>>      may help.
>>
>>  (3) Follow the style of existing commits when giving a title to
>>      your patch.
>>
>>      "git shortlog --since=18.months --no-merges git-p4.py" may
>>      help you notice "git-p4: do this thing" is the common way to
>>      title "git p4" patches.
>>
>>  (4) Justify why your change is a good thing in your log message.
>>      What you did, i.e. "list p4 jobs when making a commit", can be
>>      seen by the patch, but readers cannot guess why you thought it
>>      is a good idea to extract "job%d" out of the P4 commit and to
>>      record them in the resulting Git commit, unless you explain
>>      things like:
>>
>>      - what goes wrong if you don't?
>>      - when would "job%d" appear in P4 commit?
>>      - is it sane to assume "job0", "job1",... appear consecutively?
>>
>>  (5) Describe what your change does clearly.  "Adding list" is not
>>      quite clear.  Where in the "git commit message" are you adding
>>      the list, and why is that location in the message the most
>>      appropriate place to add it?
>
> Additionally, it would be very useful to add a test case (see t/t98*).
> There are quite a few test cases for git-p4, and they make maintenance
> a lot easier.
>
> Some documentation (Documentation/git-p4.txt) would also be useful.
>
> Thanks
> Luke

Re: [PATCH] Adding list of p4 jobs to git commit message

[Lars Schneider]

Hi Jan,

Please look closely at the Travis CI output:

non-executable tests: t9829-git-p4-jobs.sh
make[1]: *** [test-lint-executable] Error 1

You haven't set the execution bit for your new test script.
Did you try to run your tests locally? If yes, then I wonder 
how it worked... On Linux/Mac you need to do the following:

chmod u+x t9829-git-p4-jobs.sh

Afterwards it should work.

Cheers,
Lars



On 16 Apr 2016, at 15:46, Jan Durovec <jan.durovec@gmail.com> wrote:

> Hi,
> 
> I tried to address all the issues and (among other things) I've added
> a new test case to t/t98 group (t9829-git-p4-jobs.sh).
> 
> Unfortunately, Travis CI build now fails with "non-executable tests:
> t9829-git-p4-jobs.sh" (https://travis-ci.org/git/git/jobs/123555944)
> 
> Can you please advise how to mark it as executable? Does it need to be
> added to some configuration file? Or am I interpreting the error
> message incorrectly?
> 
> Regards,
> Jan
> 
> On Sat, Apr 16, 2016 at 12:10 AM, Luke Diamand <luke@diamand.org> wrote:
>> On 15 April 2016 at 21:27, Junio C Hamano <gitster@pobox.com> wrote:
>>> Jan Durovec <jan.durovec@gmail.com> writes:
>>> 
>>>> ---
>>> 
>>> A few issues.  Please:
>>> 
>>> (1) Sign-off your work.
>>> 
>>> (2) Try to find those who are familiar with the area and Cc them.
>>> 
>>>     "git shortlog -s -n --since=18.months --no-merges git-p4.py"
>>>     may help.
>>> 
>>> (3) Follow the style of existing commits when giving a title to
>>>     your patch.
>>> 
>>>     "git shortlog --since=18.months --no-merges git-p4.py" may
>>>     help you notice "git-p4: do this thing" is the common way to
>>>     title "git p4" patches.
>>> 
>>> (4) Justify why your change is a good thing in your log message.
>>>     What you did, i.e. "list p4 jobs when making a commit", can be
>>>     seen by the patch, but readers cannot guess why you thought it
>>>     is a good idea to extract "job%d" out of the P4 commit and to
>>>     record them in the resulting Git commit, unless you explain
>>>     things like:
>>> 
>>>     - what goes wrong if you don't?
>>>     - when would "job%d" appear in P4 commit?
>>>     - is it sane to assume "job0", "job1",... appear consecutively?
>>> 
>>> (5) Describe what your change does clearly.  "Adding list" is not
>>>     quite clear.  Where in the "git commit message" are you adding
>>>     the list, and why is that location in the message the most
>>>     appropriate place to add it?
>> 
>> Additionally, it would be very useful to add a test case (see t/t98*).
>> There are quite a few test cases for git-p4, and they make maintenance
>> a lot easier.
>> 
>> Some documentation (Documentation/git-p4.txt) would also be useful.
>> 
>> Thanks
>> Luke
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html