From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Miguel Torroja <miguel.torroja@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH 1/2] fast-export: deletion action first
Date: Tue, 25 Apr 2017 01:58:17 -0400 [thread overview]
Message-ID: <20170425055817.codq2q3fd54uebfx@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqy3upuk4o.fsf@gitster.mtv.corp.google.com>
On Mon, Apr 24, 2017 at 10:33:11PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Anyway. I don't think Miguel's patch needs to solve all of the lingering
> > rename cases. But I am curious whether it makes some rename cases worse,
> > because the depth-sorting was kicking in before and making them work.
>
> I agree with you on both counts, and I care more about the second
> sentence, not just "am curious", but "am worried". I am not sure
> that this patch is safe---it looked more like robbing peter to pay
> paul or the other way around. Fixing for one class of breakage
> without regressing is one thing and it is perfectly fine to leave
> some already broken case broken with such a fix. Claiming to fix
> one class and breaking other class that was happily working is quite
> different, and that is where my "Wait, we also allow renames?" comes
> from.
Yeah, I don't disagree. I am just curious first, then worried second. :)
If I had to choose, though, I'd rather see the order be reliable for the
no-renames case. IOW, if we must rob one peter, I'd rather it be the
renames, which already have tons of corner cases (and which I do not
think can be plugged for a reader which depends on the order of the
entries; the dependencies can be cycles).
Of course if we can make it work correctly in all of the non-cyclical
cases, all the better.
-Peff
next prev parent reply other threads:[~2017-04-25 5:58 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-25 0:12 [PATCH 1/2] fast-export: deletion action first Miguel Torroja
2017-04-25 0:12 ` [PATCH 2/2] fast-export: DIFF_STATUS_RENAMED instead of 'R' Miguel Torroja
2017-04-25 3:29 ` [PATCH 1/2] fast-export: deletion action first Jeff King
2017-04-25 4:24 ` Junio C Hamano
2017-04-25 4:46 ` Jeff King
2017-04-25 5:33 ` Junio C Hamano
2017-04-25 5:58 ` Jeff King [this message]
2017-05-04 21:36 ` [PATCH] " Miguel Torroja
2017-05-04 21:45 ` miguel torroja
-- strict thread matches above, loose matches on Subject: below --
2017-04-25 0:04 [PATCH 1/2] " Miguel Torroja
2017-04-24 23:04 Miguel Torroja
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170425055817.codq2q3fd54uebfx@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=miguel.torroja@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).