[Added area experts of git-p4 to Cc.]

Matt Arsenault <arse...@gmail.com> writes:

> From 425e4dc6992d07aa00039c5bb8e8c76def591fd3 Mon Sep 17 00:00:00 2001
> From: Matt Arsenault <arse...@gmail.com>
> Date: Sat, 20 Oct 2012 18:48:45 -0700
> Subject: [PATCH] git-p4: Fix not using -s option to describe

Please do not include these four lines in the body of the message.
The "Subject: " line is there so that you can copy & paste to your
MUA (instead of having to type something different from scratch),
and we use the authorship From/Date from your e-mail message.

>
> This solves errors in some cases when syncing renamed files.

Can you be a bit more descriptive?  What are "errors in some case"?

A good rule of thumb is to imagine a user of this command who is
having a problem who reads this commit log message.  Does the
symptom described to have fixed by the commit detailed enough to
allow her to tell if this change may potentially fix the problem she
is having?

> Other places where describe is used use the -s, except this one.

This makes it sound as if the changed line was an odd-man out among
many others, and it is clear we always want "describe -s" throughout
this command.  But my "git grep 'p4.*describe'" shows there are two
places, one with "-s" and one without.

What are returned from this invocation of p4Cmd() seem to be passed
to self.splitFilesIntoBranches() as its commit[] and the fields
'depotFile#', 'rev#', 'action#' and 'type#' are looked at in the
method, and then to self.commit() as its details[], for fields
'time', 'user', 'change', 'desc', and 'options'.

What field, if any, gets wrong value when "-s" is not used?  Or
perhaps some fields are omitted incorrectly?  What field are we
getting out of p4Cmd() that we are not using in the existing
callchain by not passing "-s" [*1*]?

In short, what I am getting at are:

 - What breaks by not passing "-s"?  What are the user visible
   symptoms?

 - Why is it a bug not to pass "-s"?  How does the bug happen?

Thanks.

[footnote]

*1*

http://www.perforce.com/perforce/r12.1/manuals/cmdref/describe.html
says that "describe -s" omits files diffs, but I do not know how it
affects "-G describe -s" output that we are reading (what fields in
the unmarshalled dict we are reading from is affected).

> Signed-off-by: Matt Arsenault <arse...@gmail.com>
> ---
>  git-p4.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 882b1bb..e203508 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2543,7 +2543,7 @@ class P4Sync(Command, P4UserMap):
>      def importChanges(self, changes):
>          cnt = 1
>          for change in changes:
> -            description = p4Cmd(["describe", str(change)])
> +            description = p4Cmd(["describe", "-s", str(change)])
>              self.updateOptionDict(description)
>  
>              if not self.silent:
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to