Re: [PATCH v13 03/14] Move libified code from builtin/apply.c to apply.{c,h}
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Sep 1, 2016, 07:46 UTC
- Message-ID
- <CAP8UFD1pKfCdX84P4iAyZdPdzWQMaNdb0Vaq5UpWy3fXJzFeWg@mail.gmail.com>
- In-Reply-To
- <CAGZ79kb0gao2KJFi9X3V2ME+1-xnbuSNhjC14Vnn3oP-1ac35A@mail.gmail.com>
On Wed, Aug 31, 2016 at 11:57 PM, Stefan Beller <sbeller@google.com> wrote:
Show 17 quoted lines
> On Sat, Aug 27, 2016 at 11:45 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> As most of the apply code in builtin/apply.c has been libified by a number of
>> previous commits, it can now be moved to apply.{c,h}, so that more code can
>> use it.
>>
>> Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>> apply.c | 4731 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> apply.h | 19 +
>> builtin/apply.c | 4733 +------------------------------------------------------
>
> I deduce by roughly the same line count in the .c files, it is just
> moving files over.
> (On my todo list I have an idea how to make reviewing patches like this easier.)Yeah, at one point I used some special options to try to get a smaller diff, but it got lost along the way.
Show 20 quoted lines
>> 3 files changed, 4751 insertions(+), 4732 deletions(-)
>>
>> diff --git a/apply.c b/apply.c
>> index 2eac3e3..7b96130 100644
>> --- a/apply.c
>> +++ b/apply.c
>> @@ -1,5 +1,23 @@
>> +/*
>> + * apply.c
>> + *
>> + * Copyright (C) Linus Torvalds, 2005
>
> We're very inconsistent with the intellectual property log.
> Sometimes we have that at the top of a file, sometimes we don't
> and rather point at the git history to find out who touched the code.
> I'd rather use the history instead of having a bunch of copyright lines.
> So maybe consider to drop this introductory comment as a
> {preparatory, follow up} cleanup?
>
> (This is also a nit that doesn't require a reroll on its own)Yeah, if people are ok with it I may od it in a follow up patch.