{"thread":{"id":"42049","subject":"[PATCH v2] git-p4: add P4 jobs to git commit message","startedAt":"2016-04-16T19:58:00Z","lastAt":"2016-04-21T07:34:49Z","messageCount":21,"participants":["Jan Durovec","Junio C Hamano","Lars Schneider","Luke Diamand"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"283657","messageId":"0102015420a6c30a-f2da55c9-1fc4-4df6-860e-228c5305f617-000000@eu-west-1.amazonses.com","threadId":"42049","inReplyTo":null,"subject":"[PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-16T19:58:00Z","receivedAt":"2016-04-16T19:58:00Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"When migrating from Perforce to git the information about P4 jobs\nassociated with P4 changelists is lost.\n\nHaving these jobs listed on messages of related git commits enables smooth\nmigration for projects that take advantage of e.g. JIRA integration\n(which uses jobs on Perforce side and parses commit messages on git side).\n\nThe jobs are added to the message in the same format as is expected when\nmigrating in the reverse direction.\n\nSigned-off-by: Jan Durovec \n---\n git-p4.py | 12 ++++++\n t/lib-git-p4.sh | 10 +++++\n t/t9829-git-p4-jobs.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 121 insertions(+)\n create mode 100755 t/t9829-git-p4-jobs.sh\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 527d44b..8f869d7 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@@ -2692,6 +2702,8 @@ def commit(self, details, files, branch, parent = \"\"):\n \n self.gitStream.write(\"data < 0:\n+ self.gitStream.write(\"\\nJobs: %s\" % (' '.join(jobs)))\n self.gitStream.write(\"\\n[git-p4: depot-paths = \\\"%s\\\": change = %s\" %\n (','.join(self.branchPrefixes), details[\"change\"]))\n if len(details['options']) > 0:\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex f9ae1d7..3907560 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -160,6 +160,16 @@ p4_add_user() {\n \tEOF\n }\n \n+p4_add_job() {\n+\tname=$1 &&\n+\tp4 job -f -i <<-EOF\n+\tJob: $name\n+\tStatus: open\n+\tUser: dummy\n+\tDescription:\n+\tEOF\n+}\n+\n retry_until_success() {\n \ttimeout=$(($(time_in_seconds) + $RETRY_TIMEOUT))\n \tuntil \"$@\" 2>/dev/null || test $(time_in_seconds) -gt $timeout\ndiff --git a/t/t9829-git-p4-jobs.sh b/t/t9829-git-p4-jobs.sh\nnew file mode 100755\nindex 0000000..2b9c98c\n--- /dev/null\n+++ b/t/t9829-git-p4-jobs.sh\n@@ -0,0 +1,99 @@\n+#!/bin/sh\n+\n+test_description='git p4 retrieve job info'\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+test_expect_success 'add p4 jobs' '\n+\t(\n+\t\tp4_add_job TESTJOB-A &&\n+\t\tp4_add_job TESTJOB-B\n+\t)\n+'\n+\n+test_expect_success 'add p4 files' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\t>file1 &&\n+\t\tp4 add file1 &&\n+\t\tp4 submit -d \"Add file 1\"\n+\t)\n+'\n+\n+test_expect_success 'check log message of changelist with no jobs' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init . &&\n+\t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n+\t\tcat >expect <<-\\EOF &&\n+Add file 1\n+[git-p4: depot-paths = \"//depot/\": change = 1]\n+\n+\t\tEOF\n+\t\tgit log --format=%B >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'add TESTJOB-A to change 1' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 fix -c 1 TESTJOB-A\n+\t)\n+'\n+\n+test_expect_success 'check log message of changelist with one job' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init . &&\n+\t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n+\t\tcat >expect <<-\\EOF &&\n+Add file 1\n+Jobs: TESTJOB-A\n+[git-p4: depot-paths = \"//depot/\": change = 1]\n+\n+\t\tEOF\n+\t\tgit log --format=%B >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'add TESTJOB-B to change 1' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 fix -c 1 TESTJOB-B\n+\t)\n+'\n+\n+test_expect_success 'check log message of changelist with more jobs' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init . &&\n+\t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n+\t\tcat >expect <<-\\EOF &&\n+Add file 1\n+Jobs: TESTJOB-A TESTJOB-B\n+[git-p4: depot-paths = \"//depot/\": change = 1]\n+\n+\t\tEOF\n+\t\tgit log --format=%B >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n\n--\nhttps://github.com/git/git/pull/225\n"},{"id":"283764","messageId":"xmqqshyi2yb7.fsf@gitster.mtv.corp.google.com","threadId":"42049","inReplyTo":"0102015420a6c30a-f2da55c9-1fc4-4df6-860e-228c5305f617-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-19T01:15:56Z","receivedAt":"2016-04-19T01:15:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Durovec writes:\n\n> When migrating from Perforce to git the information about P4 jobs\n> associated with P4 changelists is lost.\n>\n> Having these jobs listed on messages of related git commits enables smooth\n> migration for projects that take advantage of e.g. JIRA integration\n> (which uses jobs on Perforce side and parses commit messages on git side).\n>\n> The jobs are added to the message in the same format as is expected when\n> migrating in the reverse direction.\n>\n> Signed-off-by: Jan Durovec \n> ---\n\nThanks for describing the change more throughly than the previous\nround.\n\nLuke, how does this one look?\n\n> git-p4.py | 12 ++++++\n> t/lib-git-p4.sh | 10 +++++\n> t/t9829-git-p4-jobs.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 121 insertions(+)\n> create mode 100755 t/t9829-git-p4-jobs.sh\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 527d44b..8f869d7 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\nI am not familiar with \"Perforce jobs\", but I assume that they are\nalways named as \"job\" + small non-negative integer in a dense way\nand it is OK for this loop to always begin at 0 and immediately stop\nwhen job + num does not exist (i.e. if job7 is missing, it is\nguaranteed that we will not see job8).\n\nShouldn't the formatting be \"job%d\" % jnum, though, as you are using\njnum as a number that begins at 0 and increments by 1?\n\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> @@ -2692,6 +2702,8 @@ def commit(self, details, files, branch, parent = \"\"):\n> \n> self.gitStream.write(\"data < self.gitStream.write(details[\"desc\"])\n> + if len(jobs) > 0:\n> + self.gitStream.write(\"\\nJobs: %s\" % (' '.join(jobs)))\n> self.gitStream.write(\"\\n[git-p4: depot-paths = \\\"%s\\\": change = %s\" %\n> (','.join(self.branchPrefixes), details[\"change\"]))\n> if len(details['options']) > 0:\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> index f9ae1d7..3907560 100644\n> --- a/t/lib-git-p4.sh\n> +++ b/t/lib-git-p4.sh\n> @@ -160,6 +160,16 @@ p4_add_user() {\n> \tEOF\n> }\n> \n> +p4_add_job() {\n\nNot a new problem in this script, but we'd prefer to spell this as\n\n p4_add_job () {\n\ni.e. a space on both sides of ().\n\n> +\tname=$1 &&\n> +\tp4 job -f -i <<-EOF\n> +\tJob: $name\n> +\tStatus: open\n> +\tUser: dummy\n> +\tDescription:\n> +\tEOF\n> +}\n\nIt may be better without $name?\n\n> +test_expect_success 'check log message of changelist with no jobs' '\n> +\tclient_view \"//depot/... //client/...\" &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\tgit init . &&\n> +\t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n> +\t\tcat >expect <<-\\EOF &&\n> +Add file 1\n> +[git-p4: depot-paths = \"//depot/\": change = 1]\n> +\n> +\t\tEOF\n\nAs you are using <<- to begin the here document, it is easier on the\neyes if you indented the text with HT, i.e.\n\n\t\tcat >expect <<-\\EOF &&\n\t\tAdd file 1\n\t\t[git-p4: depot-paths = \"//depot/\": change = 1]\n\n\t\tEOF\n\nI won't repeat the same for other instances below.\n\nThanks.\n"},{"id":"283773","messageId":"80FF3761-91CC-41B0-B3A1-0DC8EC348D0D@gmail.com","threadId":"42049","inReplyTo":"0102015420a6c30a-f2da55c9-1fc4-4df6-860e-228c5305f617-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-19T07:34:27Z","receivedAt":"2016-04-19T07:34:27Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 16 Apr 2016, at 21:58, Jan Durovec wrote:\n\n> When migrating from Perforce to git the information about P4 jobs\n> associated with P4 changelists is lost.\n> \n> Having these jobs listed on messages of related git commits enables smooth\n> migration for projects that take advantage of e.g. JIRA integration\n> (which uses jobs on Perforce side and parses commit messages on git side).\n> \n> The jobs are added to the message in the same format as is expected when\n> migrating in the reverse direction.\n> \n> Signed-off-by: Jan Durovec \n> ---\n> git-p4.py | 12 ++++++\n> t/lib-git-p4.sh | 10 +++++\n> t/t9829-git-p4-jobs.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 121 insertions(+)\n> create mode 100755 t/t9829-git-p4-jobs.sh\n> \n> diff --git a/git-p4.py b/git-p4.py\n> index 527d44b..8f869d7 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]\nI tried to use the new format string syntax in the past. See:\nhttps://pyformat.info/\nhttps://docs.python.org/2/library/string.html#format-string-syntax\n\n'job{}'.format(jnum)\n\n@Luke: What do you prefer going forward?\n\n\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> @@ -2692,6 +2702,8 @@ def commit(self, details, files, branch, parent = \"\"):\n> \n> self.gitStream.write(\"data < self.gitStream.write(details[\"desc\"])\n> + if len(jobs) > 0:\n> + self.gitStream.write(\"\\nJobs: %s\" % (' '.join(jobs)))\nYou could use the new string syntax here, too. \nHow long is an avg job string? Could it make sense to write them into\nindividual lines? Would it makes sense to add an extra new line after\nthe commit message? I assume not because your commit messages says\n\"same format as is expected when migrating\"...\n\n\n> self.gitStream.write(\"\\n[git-p4: depot-paths = \\\"%s\\\": change = %s\" %\n> (','.join(self.branchPrefixes), details[\"change\"]))\n> if len(details['options']) > 0:\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> index f9ae1d7..3907560 100644\n> --- a/t/lib-git-p4.sh\n> +++ b/t/lib-git-p4.sh\n> @@ -160,6 +160,16 @@ p4_add_user() {\n> \tEOF\n> }\n> \n> +p4_add_job() {\n> +\tname=$1 &&\n> +\tp4 job -f -i <<-EOF\n> +\tJob: $name\n> +\tStatus: open\n> +\tUser: dummy\n> +\tDescription:\n> +\tEOF\n> +}\n> +\n> retry_until_success() {\n> \ttimeout=$(($(time_in_seconds) + $RETRY_TIMEOUT))\n> \tuntil \"$@\" 2>/dev/null || test $(time_in_seconds) -gt $timeout\n> diff --git a/t/t9829-git-p4-jobs.sh b/t/t9829-git-p4-jobs.sh\n> new file mode 100755\n> index 0000000..2b9c98c\n> --- /dev/null\n> +++ b/t/t9829-git-p4-jobs.sh\n> @@ -0,0 +1,99 @@\n> +#!/bin/sh\n> +\n> +test_description='git p4 retrieve job info'\n> +\n> +. ./lib-git-p4.sh\n> +\n> +test_expect_success 'start p4d' '\n> +\tstart_p4d\n> +'\n> +\n> +test_expect_success 'add p4 jobs' '\n> +\t(\n> +\t\tp4_add_job TESTJOB-A &&\n> +\t\tp4_add_job TESTJOB-B\n> +\t)\n> +'\n> +\n> +test_expect_success 'add p4 files' '\n> +\tclient_view \"//depot/... //client/...\" &&\n> +\t(\n> +\t\tcd \"$cli\" &&\n> +\t\t>file1 &&\n> +\t\tp4 add file1 &&\n> +\t\tp4 submit -d \"Add file 1\"\n> +\t)\n> +'\n> +\n> +test_expect_success 'check log message of changelist with no jobs' '\n> +\tclient_view \"//depot/... //client/...\" &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\tgit init . &&\n> +\t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n> +\t\tcat >expect <<-\\EOF &&\n> +Add file 1\n> +[git-p4: depot-paths = \"//depot/\": change = 1]\n> +\n> +\t\tEOF\n> +\t\tgit log --format=%B >actual &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n> +test_expect_success 'add TESTJOB-A to change 1' '\n> +\t(\n> +\t\tcd \"$cli\" &&\n> +\t\tp4 fix -c 1 TESTJOB-A\n> +\t)\n> +'\n> +\n> +test_expect_success 'check log message of changelist with one job' '\n> +\tclient_view \"//depot/... //client/...\" &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\tgit init . &&\n> +\t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n> +\t\tcat >expect <<-\\EOF &&\n> +Add file 1\n> +Jobs: TESTJOB-A\n> +[git-p4: depot-paths = \"//depot/\": change = 1]\n> +\n> +\t\tEOF\n> +\t\tgit log --format=%B >actual &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n> +test_expect_success 'add TESTJOB-B to change 1' '\n> +\t(\n> +\t\tcd \"$cli\" &&\n> +\t\tp4 fix -c 1 TESTJOB-B\n> +\t)\n> +'\n> +\n> +test_expect_success 'check log message of changelist with more jobs' '\n> +\tclient_view \"//depot/... //client/...\" &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\tgit init . &&\n> +\t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n> +\t\tcat >expect <<-\\EOF &&\n> +Add file 1\n> +Jobs: TESTJOB-A TESTJOB-B\n> +[git-p4: depot-paths = \"//depot/\": change = 1]\n> +\n> +\t\tEOF\n> +\t\tgit log --format=%B >actual &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n> +test_expect_success 'kill p4d' '\n> +\tkill_p4d\n> +'\n> +\n> +test_done\n> \n> --\n> https://github.com/git/git/pull/225\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"},{"id":"283774","messageId":"CAE5ih7-2mefGwfXRhvQZJFPD4QYAzZ1jYG82s6cnDzWVCiDS8w@mail.gmail.com","threadId":"42049","inReplyTo":"xmqqshyi2yb7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-04-19T08:13:37Z","receivedAt":"2016-04-19T08:13:37Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 19 April 2016 at 02:15, Junio C Hamano wrote:\n> Jan Durovec writes:\n>\n>> When migrating from Perforce to git the information about P4 jobs\n>> associated with P4 changelists is lost.\n>>\n>> Having these jobs listed on messages of related git commits enables smooth\n>> migration for projects that take advantage of e.g. JIRA integration\n>> (which uses jobs on Perforce side and parses commit messages on git side).\n>>\n>> The jobs are added to the message in the same format as is expected when\n>> migrating in the reverse direction.\n>>\n>> Signed-off-by: Jan Durovec \n>> ---\n>\n> Thanks for describing the change more throughly than the previous\n> round.\n>\n> Luke, how does this one look?\n>\n>> git-p4.py | 12 ++++++\n>> t/lib-git-p4.sh | 10 +++++\n>> t/t9829-git-p4-jobs.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++\n>> 3 files changed, 121 insertions(+)\n>> create mode 100755 t/t9829-git-p4-jobs.sh\n>>\n>> diff --git a/git-p4.py b/git-p4.py\n>> index 527d44b..8f869d7 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>\n> I am not familiar with \"Perforce jobs\", but I assume that they are\n> always named as \"job\" + small non-negative integer in a dense way\n> and it is OK for this loop to always begin at 0 and immediately stop\n> when job + num does not exist (i.e. if job7 is missing, it is\n> guaranteed that we will not see job8).\n\nThis is OK - P4 jobs have arbitrary names, but this code is just\nextracting an array of them from the commit by index.\n\n>\n> Shouldn't the formatting be \"job%d\" % jnum, though, as you are using\n> jnum as a number that begins at 0 and increments by 1?\n\nPython seems to handle this by turning jnum into a string.\n\n>\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>> @@ -2692,6 +2702,8 @@ def commit(self, details, files, branch, parent = \"\"):\n>>\n>> self.gitStream.write(\"data <> self.gitStream.write(details[\"desc\"])\n>> + if len(jobs) > 0:\n>> + self.gitStream.write(\"\\nJobs: %s\" % (' '.join(jobs)))\n>> self.gitStream.write(\"\\n[git-p4: depot-paths = \\\"%s\\\": change = %s\" %\n>> (','.join(self.branchPrefixes), details[\"change\"]))\n>> if len(details['options']) > 0:\n>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n>> index f9ae1d7..3907560 100644\n>> --- a/t/lib-git-p4.sh\n>> +++ b/t/lib-git-p4.sh\n>> @@ -160,6 +160,16 @@ p4_add_user() {\n>> EOF\n>> }\n>>\n>> +p4_add_job() {\n>\n> Not a new problem in this script, but we'd prefer to spell this as\n>\n> p4_add_job () {\n>\n> i.e. a space on both sides of ().\n>\n>> + name=$1 &&\n>> + p4 job -f -i <<-EOF\n>> + Job: $name\n>> + Status: open\n>> + User: dummy\n>> + Description:\n>> + EOF\n>> +}\n>\n> It may be better without $name?\n>\n>> +test_expect_success 'check log message of changelist with no jobs' '\n>> + client_view \"//depot/... //client/...\" &&\n>> + test_when_finished cleanup_git &&\n>> + (\n>> + cd \"$git\" &&\n>> + git init . &&\n>> + git p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n>> + cat >expect <<-\\EOF &&\n>> +Add file 1\n>> +[git-p4: depot-paths = \"//depot/\": change = 1]\n>> +\n>> + EOF\n>\n> As you are using <<- to begin the here document, it is easier on the\n> eyes if you indented the text with HT, i.e.\n>\n> cat >expect <<-\\EOF &&\n> Add file 1\n> [git-p4: depot-paths = \"//depot/\": change = 1]\n>\n> EOF\n>\n> I won't repeat the same for other instances below.\n>\n> Thanks.\n\nModulo Junio's other comments, this seems fine to me. I tried it out\non a scratch repo and it works very nicely, thanks!\n\nLuke\n"},{"id":"283802","messageId":"xmqqfuuh35v5.fsf@gitster.mtv.corp.google.com","threadId":"42049","inReplyTo":"CAE5ih7-2mefGwfXRhvQZJFPD4QYAzZ1jYG82s6cnDzWVCiDS8w@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-19T16:45:02Z","receivedAt":"2016-04-19T16:45:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand writes:\n\n>> I am not familiar with \"Perforce jobs\", but I assume that they are\n>> always named as \"job\" + small non-negative integer in a dense way\n>> and it is OK for this loop to always begin at 0 and immediately stop\n>> when job + num does not exist (i.e. if job7 is missing, it is\n>> guaranteed that we will not see job8).\n>\n> This is OK - P4 jobs have arbitrary names, but this code is just\n> extracting an array of them from the commit by index.\n\nAh, thanks, that is what I was missing and this part of the code\nmakes perfect sense to me now.\n"},{"id":"283805","messageId":"CABEqOBxZkYTm7_m-Eeq-acN=Nse1vLGk8Gm44BihVGi27KaGiw@mail.gmail.com","threadId":"42049","inReplyTo":"xmqqfuuh35v5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-19T16:56:10Z","receivedAt":"2016-04-19T16:56:10Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"Hi,\n\ngiven the fact that the rest of the code just follows existing\nsource code style, i.e.\n\n* using %s not %d to add number to string (see git-p4.py:2301)\n* no space between function name and parentheses (see all functions\n in t/lib-git-p4.sh)\n* no tab when specifying in-line expected output (see t/t9826...)\n\n...is there anything left to fix in this patch or is it good as is?\n\nI.e. would you prefer me to change the code mentioned above at the cost\nof code style consistency?\n\nIs there something else that I have missed in my enumeration?\n\nRegards,\nJan\n\nOn Tue, Apr 19, 2016 at 6:45 PM, Junio C Hamano wrote:\n> Luke Diamand writes:\n>\n>>> I am not familiar with \"Perforce jobs\", but I assume that they are\n>>> always named as \"job\" + small non-negative integer in a dense way\n>>> and it is OK for this loop to always begin at 0 and immediately stop\n>>> when job + num does not exist (i.e. if job7 is missing, it is\n>>> guaranteed that we will not see job8).\n>>\n>> This is OK - P4 jobs have arbitrary names, but this code is just\n>> extracting an array of them from the commit by index.\n>\n> Ah, thanks, that is what I was missing and this part of the code\n> makes perfect sense to me now.\n"},{"id":"283814","messageId":"xmqq7fft32y9.fsf@gitster.mtv.corp.google.com","threadId":"42049","inReplyTo":"CABEqOBxZkYTm7_m-Eeq-acN=Nse1vLGk8Gm44BihVGi27KaGiw@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-19T17:47:58Z","receivedAt":"2016-04-19T17:47:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Durovec writes:\n\n> given the fact that the rest of the code just follows existing\n> source code style, i.e.\n>\n> * using %s not %d to add number to string (see git-p4.py:2301)\n\nThis one I do not care too deeply about, as formatting anything that\ncan be formatted via '%s' could just be more Pythonic style, in\nwhich case \"%s\" is perfectly fine. It just didn't look kosher to my\nC trained eyes, that's all.\n\n> * no space between function name and parentheses (see all functions\n> in t/lib-git-p4.sh)\n\nI thought I said \"Not a new issue, but...\" to this one, and it\nappears that leaving <<- here-doc unindented, which is stupid as\nthat shows the person who is writing the here-doc does not know what\nthe dash s/he is writing means at all, is also not a new issue.\n\n> * no tab when specifying in-line expected output (see t/t9826...)\n\n> ...is there anything left to fix in this patch or is it good as is?\n>\n> I.e. would you prefer me to change the code mentioned above at the cost\n> of code style consistency?\n\nNot really.\n\nIf you really want to know the preference, we prefer a preliminary\nclean-up patch to correct existing style issues, followed by a new\nfeature patch that builds on the cleaned up codebase.\n"},{"id":"283815","messageId":"xmqq37qh32q7.fsf@gitster.mtv.corp.google.com","threadId":"42049","inReplyTo":"xmqqshyi2yb7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-19T17:52:48Z","receivedAt":"2016-04-19T17:52:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Not a new problem in this script, but we'd prefer to spell this as\n>\n> p4_add_job () {\n>\n> i.e. a space on both sides of ().\n>\n>> +\tname=$1 &&\n>> +\tp4 job -f -i <<-EOF\n>> +\tJob: $name\n>> +\tStatus: open\n>> +\tUser: dummy\n>> +\tDescription:\n>> +\tEOF\n>> +}\n>\n> It may be better without $name?\n\nJust so that I won't get misunderstood, with this I do not mean\n\"Job: $name\" line does not have to be there. I meant that there is\nno need to use name variable in this function; just writing $1\ninstead of $name there is better, as $name is not a function local\nvariable in POSIX shells.\n"},{"id":"283819","messageId":"CABEqOBwqW+BO4rtOx4ax35VacE4RZhpo_1pbqzTP_EGiSWasiQ@mail.gmail.com","threadId":"42049","inReplyTo":"xmqq7fft32y9.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-19T18:04:23Z","receivedAt":"2016-04-19T18:04:23Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"Would it be acceptable the other way around? I.e. this patch followed\nby the one that fixes code style (once this gets merged)?\n\nReason being that I don't know how to use submitGit to generate a patch\nagainst a state that is not already in git repo (ie. based on another\npatch).\n\nIn the following patch I'll\n* add spaces before () for functions in t/lib-git-p4.sh\n* remove name local variable in p4_add_job/user in t/lib-git-p4.sh\n* fix t/t98* leading tabs where <<- is used\n\nOn Tue, Apr 19, 2016 at 7:47 PM, Junio C Hamano wrote:\n> Jan Durovec writes:\n>\n>> given the fact that the rest of the code just follows existing\n>> source code style, i.e.\n>>\n>> * using %s not %d to add number to string (see git-p4.py:2301)\n>\n> This one I do not care too deeply about, as formatting anything that\n> can be formatted via '%s' could just be more Pythonic style, in\n> which case \"%s\" is perfectly fine. It just didn't look kosher to my\n> C trained eyes, that's all.\n>\n>> * no space between function name and parentheses (see all functions\n>> in t/lib-git-p4.sh)\n>\n> I thought I said \"Not a new issue, but...\" to this one, and it\n> appears that leaving <<- here-doc unindented, which is stupid as\n> that shows the person who is writing the here-doc does not know what\n> the dash s/he is writing means at all, is also not a new issue.\n>\n>> * no tab when specifying in-line expected output (see t/t9826...)\n>\n>> ...is there anything left to fix in this patch or is it good as is?\n>>\n>> I.e. would you prefer me to change the code mentioned above at the cost\n>> of code style consistency?\n>\n> Not really.\n>\n> If you really want to know the preference, we prefer a preliminary\n> clean-up patch to correct existing style issues, followed by a new\n> feature patch that builds on the cleaned up codebase.\n>\n"},{"id":"283824","messageId":"xmqqh9ex1lsy.fsf@gitster.mtv.corp.google.com","threadId":"42049","inReplyTo":"CABEqOBwqW+BO4rtOx4ax35VacE4RZhpo_1pbqzTP_EGiSWasiQ@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-19T18:43:41Z","receivedAt":"2016-04-19T18:43:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Durovec writes:\n\n> On Tue, Apr 19, 2016 at 7:47 PM, Junio C Hamano wrote:\n>>\n>> If you really want to know the preference, we prefer a preliminary\n>> clean-up patch to correct existing style issues, followed by a new\n>> feature patch that builds on the cleaned up codebase.\n>\n> Would it be acceptable the other way around? I.e. this patch followed\n> by the one that fixes code style (once this gets merged)?\n\nThe reason I said \"preference\" and not \"requirement\" is because the\nanswer to that question is \"It depends.\"\n\nWhen the primary change is something small and urgent (e.g. an\nimportant bugfix that needs to be merged down to 'maint' and older),\nwe typically do not want to keep the fix waiting for preliminary\nclean-up patch to be reviewed. Instead, we'd say \"let's fix it\nfirst on top of a known-to-be-crappy codebase, and postpone the\nclean-up until the dust settles\".\n\n>From experience, we already know that sometimes clean-up happens,\nbut often it does not, when we take this route, and it harms the\nlong-term code health, but we just say an urgent fix is more\nimportant than piling a bit more cruft on the existing technical\ndebt.\n\nAnd that experience is why we say \"preference is to have preliminary\nclean-up first\".\n\n> Reason being that I don't know how to use submitGit to generate a patch\n> against a state that is not already in git repo (ie. based on another\n> patch).\n\nAny submitGit users? I think it lets you throw multiple-patch\nseries just fine. In this case, you'd prepare a two patch series on\na branch, 1/2 being the clean-up and 2/2 being the new feature, and\nif you give that branch to submitGit as a whole it should do the\nright thing, I'd imagine.\n"},{"id":"283825","messageId":"CABEqOBxkHstqRHFUYF7=eComB-HwUGwi0tpWbhvUuKiny-=Vyw@mail.gmail.com","threadId":"42049","inReplyTo":"xmqqh9ex1lsy.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-19T18:50:44Z","receivedAt":"2016-04-19T18:50:44Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"> Any submitGit users? I think it lets you throw multiple-patch\n> series just fine. In this case, you'd prepare a two patch series on\n> a branch, 1/2 being the clean-up and 2/2 being the new feature, and\n> if you give that branch to submitGit as a whole it should do the\n> right thing, I'd imagine.\n\nHm... I'll see what it does with a pull request that has 2 commits.\n"},{"id":"283844","messageId":"CABEqOBxY61yObr0FeUxPYxc6C+xvde1LOS7zS_dHpBqwemJ+dQ@mail.gmail.com","threadId":"42049","inReplyTo":"CABEqOBxkHstqRHFUYF7=eComB-HwUGwi0tpWbhvUuKiny-=Vyw@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-19T20:28:58Z","receivedAt":"2016-04-19T20:28:58Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"Huh... seems that it works :)\n\nv3 sent in 2 parts\n\nOn Tue, Apr 19, 2016 at 8:50 PM, Jan Durovec wrote:\n>> Any submitGit users? I think it lets you throw multiple-patch\n>> series just fine. In this case, you'd prepare a two patch series on\n>> a branch, 1/2 being the clean-up and 2/2 being the new feature, and\n>> if you give that branch to submitGit as a whole it should do the\n>> right thing, I'd imagine.\n>\n> Hm... I'll see what it does with a pull request that has 2 commits.\n"},{"id":"283850","messageId":"xmqqd1plz4p5.fsf@gitster.mtv.corp.google.com","threadId":"42049","inReplyTo":"CABEqOBxY61yObr0FeUxPYxc6C+xvde1LOS7zS_dHpBqwemJ+dQ@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-19T21:09:10Z","receivedAt":"2016-04-19T21:09:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Durovec writes:\n\n> On Tue, Apr 19, 2016 at 8:50 PM, Jan Durovec wrote:\n>>> Any submitGit users? I think it lets you throw multiple-patch\n>>> series just fine. In this case, you'd prepare a two patch series on\n>>> a branch, 1/2 being the clean-up and 2/2 being the new feature, and\n>>> if you give that branch to submitGit as a whole it should do the\n>>> right thing, I'd imagine.\n>>\n>> Hm... I'll see what it does with a pull request that has 2 commits.\n>\n> Huh... seems that it works :)\n>\n> v3 sent in 2 parts\n\nThanks.\n\nFor a series this small it does not matter, but anything longer it\nwould be easier to review with a cover letter (i.e. [PATCH 0/N]). I\ndo not know if submitGit lets us do that, though.\n"},{"id":"283852","messageId":"CABEqOBx4vCUpYTGYM9VF6QHxGGgQSG5APSHymtbV7uVxAbU+ow@mail.gmail.com","threadId":"42049","inReplyTo":"xmqqd1plz4p5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-19T21:25:05Z","receivedAt":"2016-04-19T21:25:05Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"There's a comment on PR itself (in addition to individual commits) so\ntheoretically it could.\n\nIt seems that for [PATCH ... n/m] e-mails the commit messages are used,\nso there's no reason why the PR comment couldn't be used for a cover\nletter.\n\nIn this case the PR comment was the same as for one of the commits.\nMaybe submitGit tried to be smart there? Or maybe it just really\ndoesn't support it (yet?).\n\nOn Tue, Apr 19, 2016 at 11:09 PM, Junio C Hamano wrote:\n> Jan Durovec writes:\n>\n>> On Tue, Apr 19, 2016 at 8:50 PM, Jan Durovec wrote:\n>>>> Any submitGit users? I think it lets you throw multiple-patch\n>>>> series just fine. In this case, you'd prepare a two patch series on\n>>>> a branch, 1/2 being the clean-up and 2/2 being the new feature, and\n>>>> if you give that branch to submitGit as a whole it should do the\n>>>> right thing, I'd imagine.\n>>>\n>>> Hm... I'll see what it does with a pull request that has 2 commits.\n>>\n>> Huh... seems that it works :)\n>>\n>> v3 sent in 2 parts\n>\n> Thanks.\n>\n> For a series this small it does not matter, but anything longer it\n> would be easier to review with a cover letter (i.e. [PATCH 0/N]). I\n> do not know if submitGit lets us do that, though.\n"},{"id":"283855","messageId":"xmqqzispxoqt.fsf@gitster.mtv.corp.google.com","threadId":"42049","inReplyTo":"CABEqOBx4vCUpYTGYM9VF6QHxGGgQSG5APSHymtbV7uVxAbU+ow@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-19T21:39:06Z","receivedAt":"2016-04-19T21:39:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Durovec writes:\n\n> On Tue, Apr 19, 2016 at 11:09 PM, Junio C Hamano wrote:\n>\n>> For a series this small it does not matter, but anything longer it\n>> would be easier to review with a cover letter (i.e. [PATCH 0/N]). I\n>> do not know if submitGit lets us do that, though.\n>\n> There's a comment on PR itself (in addition to individual commits) so\n> theoretically it could.\n>\n> It seems that for [PATCH ... n/m] e-mails the commit messages are used,\n> so there's no reason why the PR comment couldn't be used for a cover\n> letter.\n>\n> In this case the PR comment was the same as for one of the commits.\n> Maybe submitGit tried to be smart there? Or maybe it just really\n> doesn't support it (yet?).\n\nIn any case, I've queued the updated ones as they looked good.\nThanks.\n\nBy the way, you may or may not have noticed that I've been\nreordering the lines of your message quoted in my responses; around\nhere, top-posting is frowned upon.\n"},{"id":"283857","messageId":"CABEqOBzXvr3Xp+XJtqjL5eY8Spy8Bi1SezCut0_QJdYnw9C4Zg@mail.gmail.com","threadId":"42049","inReplyTo":"xmqqzispxoqt.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-19T21:44:10Z","receivedAt":"2016-04-19T21:44:10Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"> By the way, you may or may not have noticed that I've been\n> reordering the lines of your message quoted in my responses; around\n> here, top-posting is frowned upon.\n\nI haven't noticed. Thanks for pointing out.\n\nAs for the submitGit cover letter I wanted to raise at least an issue\n(if not create a fix itself) but it seems to be raised already as\nhttps://github.com/rtyley/submitgit/issues/9\n"},{"id":"283912","messageId":"CAE5ih78Cj+-BrE3WrGF3Wo5Nr+8PwY+pgBWee5VzerzhytBMWg@mail.gmail.com","threadId":"42049","inReplyTo":"CABEqOBzXvr3Xp+XJtqjL5eY8Spy8Bi1SezCut0_QJdYnw9C4Zg@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-04-20T07:56:55Z","receivedAt":"2016-04-20T07:56:55Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 19 April 2016 at 22:44, Jan Durovec wrote:\n>> By the way, you may or may not have noticed that I've been\n>> reordering the lines of your message quoted in my responses; around\n>> here, top-posting is frowned upon.\n>\n> I haven't noticed. Thanks for pointing out.\n>\n> As for the submitGit cover letter I wanted to raise at least an issue\n> (if not create a fix itself) but it seems to be raised already as\n> https://github.com/rtyley/submitgit/issues/9\n\nTo be honest, it would be just as easy to learn how to use git\nformat-patch and git send-email. It makes your head hurt a bit the\nfirst few times you use it, but it's pretty straightforward.\n\nAnd it also means that if there's a zombie apocalypse, and github gets\noverrun, you can still exchange patches to your anti-virus and save\nhumanity from extinction.\n\nOf course, if you don't care about the future of the human race, then\nthat's up to you.... :-)\n\nLuke\n"},{"id":"283913","messageId":"CAE5ih7_7TeJDekoubzJ=ZPT_JUwQVyGczDt5yORZES89Qo=VjA@mail.gmail.com","threadId":"42049","inReplyTo":"xmqqzispxoqt.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-04-20T07:58:58Z","receivedAt":"2016-04-20T07:58:58Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 19 April 2016 at 22:39, Junio C Hamano wrote:\n> Jan Durovec writes:\n>\n>> On Tue, Apr 19, 2016 at 11:09 PM, Junio C Hamano wrote:\n>>\n>>> For a series this small it does not matter, but anything longer it\n>>> would be easier to review with a cover letter (i.e. [PATCH 0/N]). I\n>>> do not know if submitGit lets us do that, though.\n>>\n>> There's a comment on PR itself (in addition to individual commits) so\n>> theoretically it could.\n>>\n>> It seems that for [PATCH ... n/m] e-mails the commit messages are used,\n>> so there's no reason why the PR comment couldn't be used for a cover\n>> letter.\n>>\n>> In this case the PR comment was the same as for one of the commits.\n>> Maybe submitGit tried to be smart there? Or maybe it just really\n>> doesn't support it (yet?).\n>\n> In any case, I've queued the updated ones as they looked good.\n> Thanks.\n\nOne thing I wondered about is whether this should be enabled by\ndefault or not. Long-time users of git-p4 might be a bit surprised to\nfind their git commits suddenly gaining an extra Job: field.\n\nLuke\n"},{"id":"283956","messageId":"xmqqtwiwwa5x.fsf@gitster.mtv.corp.google.com","threadId":"42049","inReplyTo":"CAE5ih7_7TeJDekoubzJ=ZPT_JUwQVyGczDt5yORZES89Qo=VjA@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-20T15:51:38Z","receivedAt":"2016-04-20T15:51:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand writes:\n\n> One thing I wondered about is whether this should be enabled by\n> default or not. Long-time users of git-p4 might be a bit surprised to\n> find their git commits suddenly gaining an extra Job: field.\n\nAhh, I didn't even wonder about but that is not because I didn't\nthink it matters.\n\nDoes this change affect reproducibility of importing the history\nfrom P4, doesn't it? Would that be a problem?\n\nHow common is it to have the \"extra\" Job: thing in the history on P4\nside? If the answer to this question is \"on rare occasions and only\nwhen there is a very good reason to have 'jobs' associated with the\nchangelist\", then the 'might be a bit surprised' brought by this\nchange can probably be explained away as \"a fix to a (design) bug\nthat used to discard crucial information\" that (unfortunately) have\nto change the resulting Git object names.\n"},{"id":"283966","messageId":"CABEqOBwGLom5n61+qnK7uhJMhVs4CeFY7aSXPm1xWvmH8RX2Lg@mail.gmail.com","threadId":"42049","inReplyTo":"CAE5ih7_7TeJDekoubzJ=ZPT_JUwQVyGczDt5yORZES89Qo=VjA@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Jan Durovec","fromEmail":"jan.durovec@gmail.com","sentAt":"2016-04-20T16:30:39Z","receivedAt":"2016-04-20T16:30:39Z","isPatch":true,"sender":{"key":"jan.durovec@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16178826?v=4"},"body":"> One thing I wondered about is whether this should be enabled by\n> default or not. Long-time users of git-p4 might be a bit surprised to\n> find their git commits suddenly gaining an extra Job: field.\n\nI thought about that too when but then I realized that there's no\nswitch for the reverse direction either. I.e. when committing to p4\nfrom git the commit messages are parsed and \"Jobs: ...\" section is\ninterpreted regardless of any switch, isn't it?\n\nThat's why I decided to keep this behaviour symmetrical. Otherwise I\nwould have reused the same switch that enables/disables job parsing\nin git->p4 direction.\n"},{"id":"284022","messageId":"CAE5ih78r=14fPdk76H=VBYbvFj7eqKrdoEEbaX9+vo+G_HREVA@mail.gmail.com","threadId":"42049","inReplyTo":"xmqqtwiwwa5x.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: add P4 jobs to git commit message","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-04-21T07:34:49Z","receivedAt":"2016-04-21T07:34:49Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 20 April 2016 at 16:51, Junio C Hamano wrote:\n> Luke Diamand writes:\n>\n>> One thing I wondered about is whether this should be enabled by\n>> default or not. Long-time users of git-p4 might be a bit surprised to\n>> find their git commits suddenly gaining an extra Job: field.\n>\n> Ahh, I didn't even wonder about but that is not because I didn't\n> think it matters.\n>\n> Does this change affect reproducibility of importing the history\n> from P4, doesn't it? Would that be a problem?\n\nIt would change the history created, but I don't see why that would be\na problem.\n\n>\n> How common is it to have the \"extra\" Job: thing in the history on P4\n> side?\n\nWhere I work currently we don't use jobs (at present). Where I worked\nbefore, jobs were created automatically to track issues in JIRA, and\nwere (supposed to be) entered into commits. It's potentially quite\nuseful so I guess might be quite widespread.\n\n> If the answer to this question is \"on rare occasions and only\n> when there is a very good reason to have 'jobs' associated with the\n> changelist\", then the 'might be a bit surprised' brought by this\n> change can probably be explained away as \"a fix to a (design) bug\n> that used to discard crucial information\" that (unfortunately) have\n> to change the resulting Git object names.\n>\n\nLuke\n"}]}