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