{"thread":{"id":"42039","subject":"[PATCH] Adding list of p4 jobs to git commit message","startedAt":"2016-04-15T19:51:10Z","lastAt":"2016-04-16T14:51:31Z","messageCount":5,"participants":["Jan Durovec","Junio C Hamano","Luke Diamand","Lars Schneider"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"283571","messageId":"010201541b7a24b8-83b0f526-2f01-40e3-afc8-d81549af70d4-000000@eu-west-1.amazonses.com","threadId":"42039","inReplyTo":null,"subject":"[PATCH] Adding list of p4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-15T19:51:10Z","receivedAt":"2016-04-15T19:51:10Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"---\n git-p4.py | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 527d44b..a81795f 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2320,6 +2320,15 @@ def extractFilesFromCommit(self, commit):\n fnum = fnum + 1\n return files\n \n+ def extractJobsFromCommit(self, commit):\n+ jobs = []\n+ jnum = 0\n+ while commit.has_key(\"job%s\" % jnum):\n+ job = commit[\"job%s\" % jnum]\n+ jobs.append(job)\n+ jnum = jnum + 1\n+ return jobs\n+\n def stripRepoPath(self, path, prefixes):\n \"\"\"When streaming files, this is called to map a p4 depot path\n to where it should go in git. The prefixes are either\n@@ -2665,6 +2674,7 @@ def hasBranchPrefix(self, path):\n def commit(self, details, files, branch, parent = \"\"):\n epoch = details[\"time\"]\n author = details[\"user\"]\n+ jobs = self.extractJobsFromCommit(details)\n \n if self.verbose:\n print('commit into {0}'.format(branch))\n@@ -2696,6 +2706,8 @@ def commit(self, details, files, branch, parent = \"\"):\n (','.join(self.branchPrefixes), details[\"change\"]))\n if len(details['options']) > 0:\n self.gitStream.write(\": options = %s\" % details['options'])\n+ if len(jobs) > 0:\n+ self.gitStream.write(\": jobs = %s\" % (','.join(jobs)))\n self.gitStream.write(\"]\\nEOT\\n\\n\")\n \n if len(parent) > 0:\n\n--\nhttps://github.com/git/git/pull/225\n"},{"id":"283579","messageId":"xmqq4mb2bot7.fsf@gitster.mtv.corp.google.com","threadId":"42039","inReplyTo":"010201541b7a24b8-83b0f526-2f01-40e3-afc8-d81549af70d4-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] Adding list of p4 jobs to git commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-15T20:27:16Z","receivedAt":"2016-04-15T20:27:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Durovec writes:\n\n> ---\n\nA few issues. Please:\n\n (1) Sign-off your work.\n\n (2) Try to find those who are familiar with the area and Cc them.\n\n \"git shortlog -s -n --since=18.months --no-merges git-p4.py\"\n may help.\n\n (3) Follow the style of existing commits when giving a title to\n your patch.\n\n \"git shortlog --since=18.months --no-merges git-p4.py\" may\n help you notice \"git-p4: do this thing\" is the common way to\n title \"git p4\" patches.\n\n (4) Justify why your change is a good thing in your log message.\n What you did, i.e. \"list p4 jobs when making a commit\", can be\n seen by the patch, but readers cannot guess why you thought it\n is a good idea to extract \"job%d\" out of the P4 commit and to\n record them in the resulting Git commit, unless you explain\n things like:\n\n - what goes wrong if you don't?\n - when would \"job%d\" appear in P4 commit?\n - is it sane to assume \"job0\", \"job1\",... appear consecutively?\n\n (5) Describe what your change does clearly. \"Adding list\" is not\n quite clear. Where in the \"git commit message\" are you adding\n the list, and why is that location in the message the most\n appropriate place to add it?\n\nThanks.\n\n> git-p4.py | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 527d44b..a81795f 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -2320,6 +2320,15 @@ def extractFilesFromCommit(self, commit):\n> fnum = fnum + 1\n> return files\n> \n> + def extractJobsFromCommit(self, commit):\n> + jobs = []\n> + jnum = 0\n> + while commit.has_key(\"job%s\" % jnum):\n> + job = commit[\"job%s\" % jnum]\n> + jobs.append(job)\n> + jnum = jnum + 1\n> + return jobs\n> +\n> def stripRepoPath(self, path, prefixes):\n> \"\"\"When streaming files, this is called to map a p4 depot path\n> to where it should go in git. The prefixes are either\n> @@ -2665,6 +2674,7 @@ def hasBranchPrefix(self, path):\n> def commit(self, details, files, branch, parent = \"\"):\n> epoch = details[\"time\"]\n> author = details[\"user\"]\n> + jobs = self.extractJobsFromCommit(details)\n> \n> if self.verbose:\n> print('commit into {0}'.format(branch))\n> @@ -2696,6 +2706,8 @@ def commit(self, details, files, branch, parent = \"\"):\n> (','.join(self.branchPrefixes), details[\"change\"]))\n> if len(details['options']) > 0:\n> self.gitStream.write(\": options = %s\" % details['options'])\n> + if len(jobs) > 0:\n> + self.gitStream.write(\": jobs = %s\" % (','.join(jobs)))\n> self.gitStream.write(\"]\\nEOT\\n\\n\")\n> \n> if len(parent) > 0:\n>\n> --\n> https://github.com/git/git/pull/225\n"},{"id":"283602","messageId":"CAE5ih79hV-76SxjW0NHjD+WsvO1s6rthM=pvqyrh10zbrFi2rQ@mail.gmail.com","threadId":"42039","inReplyTo":"xmqq4mb2bot7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Adding list of p4 jobs to git commit message","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-04-15T22:10:30Z","receivedAt":"2016-04-15T22:10:30Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 15 April 2016 at 21:27, Junio C Hamano wrote:\n> Jan Durovec writes:\n>\n>> ---\n>\n> A few issues. Please:\n>\n> (1) Sign-off your work.\n>\n> (2) Try to find those who are familiar with the area and Cc them.\n>\n> \"git shortlog -s -n --since=18.months --no-merges git-p4.py\"\n> may help.\n>\n> (3) Follow the style of existing commits when giving a title to\n> your patch.\n>\n> \"git shortlog --since=18.months --no-merges git-p4.py\" may\n> help you notice \"git-p4: do this thing\" is the common way to\n> title \"git p4\" patches.\n>\n> (4) Justify why your change is a good thing in your log message.\n> What you did, i.e. \"list p4 jobs when making a commit\", can be\n> seen by the patch, but readers cannot guess why you thought it\n> is a good idea to extract \"job%d\" out of the P4 commit and to\n> record them in the resulting Git commit, unless you explain\n> things like:\n>\n> - what goes wrong if you don't?\n> - when would \"job%d\" appear in P4 commit?\n> - is it sane to assume \"job0\", \"job1\",... appear consecutively?\n>\n> (5) Describe what your change does clearly. \"Adding list\" is not\n> quite clear. Where in the \"git commit message\" are you adding\n> the list, and why is that location in the message the most\n> appropriate place to add it?\n\nAdditionally, it would be very useful to add a test case (see t/t98*).\nThere are quite a few test cases for git-p4, and they make maintenance\na lot easier.\n\nSome documentation (Documentation/git-p4.txt) would also be useful.\n\nThanks\nLuke\n"},{"id":"283646","messageId":"CABEqOBwcb1BAmvcBLow38BnQoxRNVYtfXzGAodUv_qwEBWRFvQ@mail.gmail.com","threadId":"42039","inReplyTo":"CAE5ih79hV-76SxjW0NHjD+WsvO1s6rthM=pvqyrh10zbrFi2rQ@mail.gmail.com","subject":"Re: [PATCH] Adding list of p4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-16T13:46:48Z","receivedAt":"2016-04-16T13:46:48Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"Hi,\n\nI tried to address all the issues and (among other things) I've added\na new test case to t/t98 group (t9829-git-p4-jobs.sh).\n\nUnfortunately, Travis CI build now fails with \"non-executable tests:\nt9829-git-p4-jobs.sh\" (https://travis-ci.org/git/git/jobs/123555944)\n\nCan you please advise how to mark it as executable? Does it need to be\nadded to some configuration file? Or am I interpreting the error\nmessage incorrectly?\n\nRegards,\nJan\n\nOn Sat, Apr 16, 2016 at 12:10 AM, Luke Diamand wrote:\n> On 15 April 2016 at 21:27, Junio C Hamano wrote:\n>> Jan Durovec writes:\n>>\n>>> ---\n>>\n>> A few issues. Please:\n>>\n>> (1) Sign-off your work.\n>>\n>> (2) Try to find those who are familiar with the area and Cc them.\n>>\n>> \"git shortlog -s -n --since=18.months --no-merges git-p4.py\"\n>> may help.\n>>\n>> (3) Follow the style of existing commits when giving a title to\n>> your patch.\n>>\n>> \"git shortlog --since=18.months --no-merges git-p4.py\" may\n>> help you notice \"git-p4: do this thing\" is the common way to\n>> title \"git p4\" patches.\n>>\n>> (4) Justify why your change is a good thing in your log message.\n>> What you did, i.e. \"list p4 jobs when making a commit\", can be\n>> seen by the patch, but readers cannot guess why you thought it\n>> is a good idea to extract \"job%d\" out of the P4 commit and to\n>> record them in the resulting Git commit, unless you explain\n>> things like:\n>>\n>> - what goes wrong if you don't?\n>> - when would \"job%d\" appear in P4 commit?\n>> - is it sane to assume \"job0\", \"job1\",... appear consecutively?\n>>\n>> (5) Describe what your change does clearly. \"Adding list\" is not\n>> quite clear. Where in the \"git commit message\" are you adding\n>> the list, and why is that location in the message the most\n>> appropriate place to add it?\n>\n> Additionally, it would be very useful to add a test case (see t/t98*).\n> There are quite a few test cases for git-p4, and they make maintenance\n> a lot easier.\n>\n> Some documentation (Documentation/git-p4.txt) would also be useful.\n>\n> Thanks\n> Luke\n"},{"id":"283647","messageId":"8D2BE57C-BE68-4014-ADED-3382DF164ECA@gmail.com","threadId":"42039","inReplyTo":"CABEqOBwcb1BAmvcBLow38BnQoxRNVYtfXzGAodUv_qwEBWRFvQ@mail.gmail.com","subject":"Re: [PATCH] Adding list of p4 jobs to git commit message","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-16T14:51:31Z","receivedAt":"2016-04-16T14:51:31Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Hi Jan,\n\nPlease look closely at the Travis CI output:\n\nnon-executable tests: t9829-git-p4-jobs.sh\nmake[1]: *** [test-lint-executable] Error 1\n\nYou haven't set the execution bit for your new test script.\nDid you try to run your tests locally? If yes, then I wonder \nhow it worked... On Linux/Mac you need to do the following:\n\nchmod u+x t9829-git-p4-jobs.sh\n\nAfterwards it should work.\n\nCheers,\nLars\n\n\n\nOn 16 Apr 2016, at 15:46, Jan Durovec wrote:\n\n> Hi,\n> \n> I tried to address all the issues and (among other things) I've added\n> a new test case to t/t98 group (t9829-git-p4-jobs.sh).\n> \n> Unfortunately, Travis CI build now fails with \"non-executable tests:\n> t9829-git-p4-jobs.sh\" (https://travis-ci.org/git/git/jobs/123555944)\n> \n> Can you please advise how to mark it as executable? Does it need to be\n> added to some configuration file? Or am I interpreting the error\n> message incorrectly?\n> \n> Regards,\n> Jan\n> \n> On Sat, Apr 16, 2016 at 12:10 AM, Luke Diamand wrote:\n>> On 15 April 2016 at 21:27, Junio C Hamano wrote:\n>>> Jan Durovec writes:\n>>> \n>>>> ---\n>>> \n>>> A few issues. Please:\n>>> \n>>> (1) Sign-off your work.\n>>> \n>>> (2) Try to find those who are familiar with the area and Cc them.\n>>> \n>>> \"git shortlog -s -n --since=18.months --no-merges git-p4.py\"\n>>> may help.\n>>> \n>>> (3) Follow the style of existing commits when giving a title to\n>>> your patch.\n>>> \n>>> \"git shortlog --since=18.months --no-merges git-p4.py\" may\n>>> help you notice \"git-p4: do this thing\" is the common way to\n>>> title \"git p4\" patches.\n>>> \n>>> (4) Justify why your change is a good thing in your log message.\n>>> What you did, i.e. \"list p4 jobs when making a commit\", can be\n>>> seen by the patch, but readers cannot guess why you thought it\n>>> is a good idea to extract \"job%d\" out of the P4 commit and to\n>>> record them in the resulting Git commit, unless you explain\n>>> things like:\n>>> \n>>> - what goes wrong if you don't?\n>>> - when would \"job%d\" appear in P4 commit?\n>>> - is it sane to assume \"job0\", \"job1\",... appear consecutively?\n>>> \n>>> (5) Describe what your change does clearly. \"Adding list\" is not\n>>> quite clear. Where in the \"git commit message\" are you adding\n>>> the list, and why is that location in the message the most\n>>> appropriate place to add it?\n>> \n>> Additionally, it would be very useful to add a test case (see t/t98*).\n>> There are quite a few test cases for git-p4, and they make maintenance\n>> a lot easier.\n>> \n>> Some documentation (Documentation/git-p4.txt) would also be useful.\n>> \n>> Thanks\n>> Luke\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"}]}