Re: [PATCHv2 0/4] Some cleanups

From: Stefan Beller <sbeller@google.com>
Date: Mar 30, 2016, 17:32 UTC
Message-ID: <CAGZ79kYwpj+NSn4yZK0tqASQ7CghAMgE6L9thVhTwuBNmFY6PA@mail.gmail.com>
In-Reply-To: <20160330172538.GC19675@sigill.intra.peff.net>

On Wed, Mar 30, 2016 at 10:25 AM, Jeff King <peff@peff.net> wrote:

Show 14 quoted lines

> On Wed, Mar 30, 2016 at 10:05:14AM -0700, Stefan Beller wrote:
>
>> v2:
>> Thanks Eric, Jeff, Junio for discussion, I picked up all hints and here
>> is a v2.
>>
>> * drop the overallocation patches (1&2)
>> * use git_config_get_string instead of its _const equivalent, such that
>>   we don't need a cast when freeing in git_config_get_notes_strategy
>> * Use strbuf_list_free instead of cooking our own.
>> * have a dedicated error exit path in bundle.c, create_bundle
>
> I'm OK with all of these as-is, though I did mention a nit in the third
> one. I also like Junio's rewrite instead of using strbuf_list_free.

I'm fine using the rewritten version instead of using strbuf_list_free. :) On the third one, there is one case, where we have

  if (..)
    return error(_(text));

and that is an exit(128); eventually.

I thought it is worth preserving the difference (as it is a faithful bug fix not a change to make it better or more uniform).

Thanks, Stefan

>
> -Peff