Re: [PATCH 0/2] git-p4: fix for handling of multiple depot paths

From: James Farwell <jfarwell@vmware.com>
Date: Dec 15, 2015, 22:56 UTC
Message-ID: <1450220213834.32062@vmware.com>
In-Reply-To: <CAE5ih7_qY5oF+UWs4gE2eHUu17pBg6TVGTUyRRRcBe12ybkw+Q@mail.gmail.com>

I'm not sure if my opinion as an outsider is of use, but since the perforce change number is monotonically increasing, my expectation as a user would be for them to be applied in order by the perforce change number. :)

- James

________________________________________
From: Luke Diamand <luke@diamand.org>
Sent: Monday, December 14, 2015 3:09 PM
To: Junio C Hamano
Cc: Git Users; James Farwell; Lars Schneider; Eric Sunshine; Sam Hocevar
Subject: Re: [PATCH 0/2] git-p4: fix for handling of multiple depot paths

Sorry - I've just run the tests, and this change causes one of the test cases in t9800-git-p4-basic.sh to fail.

It looks like the test case makes an assumption about who wins if two P4 depots have changes to files that end up in the same place, and this change reverses the order. It may actually be fine, but it needs to be thought about a bit.

Sam - do you have any thoughts on this?

Thanks Luke

On 14 December 2015 at 22:06, Junio C Hamano <gitster@pobox.com> wrote:

Show 20 quoted lines

> Luke Diamand <luke@diamand.org> writes:
>
>> On 14 December 2015 at 19:16, Junio C Hamano <gitster@pobox.com> wrote:
>>> Luke Diamand <luke@diamand.org> writes:
>>>
>>>> Having just fixed this, I've now just spotted that Sam Hocevar's fix
>>>> to reduce the number of P4 transactions also fixes it:
>>>>
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mail-2Darchive.com_git-2540vger.kernel.org_msg81880.html&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=wkCayFhpIBdAOEa7tZDTcd1weqwtiFMEIQTL-WQPwC4&m=q8dsOAHvUiDzzPNGRAfMMrcXstxNlI-v7I_03uEL1e8&s=C8wVLMC-iU7We0r36sxOuu920ZjZYdpy7ysNi_5PYv8&e=
>>>>
>>>> That seems like a cleaner fix.
>>>
>>> Hmm, do you mean I should ignore this series and take the other one,
>>> take only 1/2 from this for tests and then both patches in the other
>>> one, or something else?
>>
>> The second of those (take only 1/2 from this for tests, and then both
>> from the other) seems like the way to go.
>
> OK.  Should I consider the two patches from Sam "Reviewed-by" you?