Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

From: Duy Nguyen <pclouds@gmail.com>
Date: May 4, 2016, 11:32 UTC
Message-ID: <CACsJy8AtEv9HHOJUDVKmO=qLkWctvS2zoGdo03jtQwyy2kKzRQ@mail.gmail.com>
In-Reply-To: <CAP8UFD3o_u48rRHP7ZOWnVH7X0O5wwM-Gk-VpH5PP_kFx19ZpQ@mail.gmail.com>

On Wed, May 4, 2016 at 5:39 PM, Christian Couder <christian.couder@gmail.com> wrote:

Show 30 quoted lines

> On Mon, Apr 25, 2016 at 3:46 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder
>> <christian.couder@gmail.com> wrote:
>>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>>> ---
>>>  apply.c         | 4678 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  apply.h         |   19 +
>>>  builtin/apply.c | 4677 ------------------------------------------------------
>>>  3 files changed, 4695 insertions(+), 4679 deletions(-)
>>
>> Maybe if you can rearrange functions in the right order in
>> builtin/apply.c first and move everything in one patch (i.e. apply.c
>> is created here), rename detection kicks in realizes apply.c is a
>> rename of (old) builtin/apply.c and only shows the diff of (new)
>> builtin/apply.c which is about 90 lines... I haven't tested if this is
>> true though.
>
> Right now I get:
>
>> git log --summary -M -C -B -1 20f1d27
> commit 20f1d274609f5dde2eaaa279e7ee79fd5ef9c849
> Author: Christian Couder <chriscool@tuxfamily.org>
> Date:   Fri Apr 22 20:55:46 2016 +0200
>
>     Move libified code from builtin/apply.c to apply.{c,h}
>
>     Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>
>  copy builtin/apply.c => apply.c (96%)
>  rewrite builtin/apply.c (96%)

Ah.. I forgot about -B to break rename pairs! This looks good. I really need to go back to diff rename hints series, so we can annotate this commit and don't have to use -B every time.

Show 6 quoted lines

> And using format-patch:
>
>> git format-patch -M -C -B -1  -o ../../patches/test-libify-apply-use-in-am/ 20f1d27
> ../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
>> wc ../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
>   5264  23426 147133

It's a lot of lines, but if you look at diff stat, it's 244 insertions and 4906 deletions. I _think_ deletions are something reviewers don't have to pay much attention to (trusting that git moves code correctly), so only 244 lines to review are really nice. But I may be wrong.

-- 
Duy