Thread (47 messages) 47 messages, 4 authors, 2017-11-26

Re: [PATCH v3 00/33] Add directory rename detection to git

From: Stefan Beller <hidden>
Date: 2017-11-22 19:24:56

On Tue, Nov 21, 2017 at 5:12 PM, Elijah Newren [off-list ref] wrote:
On Tue, Nov 21, 2017 at 4:42 PM, Stefan Beller [off-list ref] wrote:
quoted
On Tue, Nov 21, 2017 at 12:00 AM, Elijah Newren [off-list ref] wrote:
quoted
This patchset introduces directory rename detection to merge-recursive; I'm
resubmitting just a few hours after my PATCHv2 because I didn't know about
the DEVELOPER=1 flag previously, and my code had a number of
warnings/errors.  I would have just submitted fixup/squash patches, but
when I checked, there sadly they cause merge conflicts when rebasing

See https://public-inbox.org/git/20171110190550.27059-1-newren@gmail.com/
for the first series, design considerations, etc, and
https://public-inbox.org/git/20171120220209.15111-1-newren@gmail.com/ for
v2.
Thanks, I'll take a look!

Protip: To make it easy for reviewers add an interdiff[1] between the different
versions of the patch series, this can be done via tbdiff[2] for example,
or in case you still have the old branch around or Junio has it queued already,
you can do a diff against that branch.
Thanks!

Interesting; tbdiff looks cool.  Junio hasn't queued this series yet,
but it's easy enough to reconstruct the old one.  It does weigh in
pretty heavy, and I'm slighly worried about gmail mangling all the
lines, but I'm going to give it a shot anyway.  If it's too mangled,
I'll try to repost using git-send-email.  It does weigh in at over
1600 lines, so it's not small.
In my first round of review I only looked over the tests to see if I'd
find the behavior intuitive, I spared the implementation, as Junio seemed
to have reviewed a couple patches of the v1 implementation.

Now I also looked over the implementation and quite like it, though
I'd be happy if others would also have a look.

All but one comment were minor style nits, which are no big deal;
the other remark that I was musing about was whether we want to use
strbufs in the new code instead of e.g. sprintfs to extend strings.
And I'd think we would want to use them unless there are compelling
reasons not to.

Thanks,
Stefan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help