Jan Durovec <jan.durovec@xxxxxxxxx> writes:
> When migrating from Perforce to git the information about P4 jobs
> associated with P4 changelists is lost.
>
> Having these jobs listed on messages of related git commits enables smooth
> migration for projects that take advantage of e.g. JIRA integration
> (which uses jobs on Perforce side and parses commit messages on git side).
>
> The jobs are added to the message in the same format as is expected when
> migrating in the reverse direction.
>
> Signed-off-by: Jan Durovec <jan.durovec@xxxxxxxxx>
> ---
Thanks for describing the change more throughly than the previous
round.
Luke, how does this one look?
> git-p4.py | 12 ++++++
> t/lib-git-p4.sh | 10 +++++
> t/t9829-git-p4-jobs.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 121 insertions(+)
> create mode 100755 t/t9829-git-p4-jobs.sh
>
> diff --git a/git-p4.py b/git-p4.py
> index 527d44b..8f869d7 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
I am not familiar with "Perforce jobs", but I assume that they are
always named as "job" + small non-negative integer in a dense way
and it is OK for this loop to always begin at 0 and immediately stop
when job + num does not exist (i.e. if job7 is missing, it is
guaranteed that we will not see job8).
Shouldn't the formatting be "job%d" % jnum, though, as you are using
jnum as a number that begins at 0 and increments by 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))
> @@ -2692,6 +2702,8 @@ def commit(self, details, files, branch, parent = ""):
>
> self.gitStream.write("data <<EOT\n")
> self.gitStream.write(details["desc"])
> + if len(jobs) > 0:
> + self.gitStream.write("\nJobs: %s" % (' '.join(jobs)))
> self.gitStream.write("\n[git-p4: depot-paths = \"%s\": change = %s" %
> (','.join(self.branchPrefixes), details["change"]))
> if len(details['options']) > 0:
> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> index f9ae1d7..3907560 100644
> --- a/t/lib-git-p4.sh
> +++ b/t/lib-git-p4.sh
> @@ -160,6 +160,16 @@ p4_add_user() {
> EOF
> }
>
> +p4_add_job() {
Not a new problem in this script, but we'd prefer to spell this as
p4_add_job () {
i.e. a space on both sides of ().
> + name=$1 &&
> + p4 job -f -i <<-EOF
> + Job: $name
> + Status: open
> + User: dummy
> + Description:
> + EOF
> +}
It may be better without $name?
> +test_expect_success 'check log message of changelist with no jobs' '
> + client_view "//depot/... //client/..." &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + git init . &&
> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
> + cat >expect <<-\EOF &&
> +Add file 1
> +[git-p4: depot-paths = "//depot/": change = 1]
> +
> + EOF
As you are using <<- to begin the here document, it is easier on the
eyes if you indented the text with HT, i.e.
cat >expect <<-\EOF &&
Add file 1
[git-p4: depot-paths = "//depot/": change = 1]
EOF
I won't repeat the same for other instances below.
Thanks.
--
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