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

From: Christian Couder <christian.couder@gmail.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Karsten Blees" <karsten.blees@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Stefan Beller" <sbeller@google.com>,
	"Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}
Date: Wed, 4 May 2016 12:39:07 +0200	[thread overview]
Message-ID: <CAP8UFD3o_u48rRHP7ZOWnVH7X0O5wwM-Gk-VpH5PP_kFx19ZpQ@mail.gmail.com> (raw)
In-Reply-To: <CACsJy8Cb20L=y_=J6S5=rngvH1n0aWvtf5eJrXP-df0aoiJyrQ@mail.gmail.com>

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%)

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
../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch

The previous patch was:

> wc ../../patches/libify-apply-use-in-am25/0078-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
  9436  41887 254435
../../patches/libify-apply-use-in-am25/0078-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch

So it is better, though not really as small as I would like.

I am also not so sure that moving everything in one patch would make
things much smaller.
By the way at one point I have to create apply.h from some of the
content of builtin/apply.c (like the struct apply_state) and I don't
know if you are talking about that too.

If you want you can try using the latest version of the series:

https://github.com/chriscool/git/commits/libify-apply-use-in-am39

Right now I think I will just use -M -C -B with format-patch for v2.

Thanks,
Christian.