Re: [PATCH 0/2] git-p4: fix for handling of multiple depot paths
I'm actually surprised that the patch changes the order at all, since
all it does is affect the decision (on a yes/no basis) to include a given
file into a changelist. I'm going to have a look at that specific unit
test, but of course as a user I'd prefer if the default behaviour could
remain the same, unless it was actually a bug.
--
Sam.
On Tue, Dec 15, 2015, James Farwell wrote:
> 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:
> > 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?
echo "creationism" | tr -d "holy godly goal"
Sam Hocevar <sam@hocevar.net>