From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Mon, 01 Aug 2011 17:55:37 GMT
Subject: Re: [PATCH 11/18] revert: Save command-line options for continuing operation
Message-ID: <CALkWK0nprofui1HyewJeCQiX+uRxABAOeFPMGNqQ+jUHtB13GA@mail.gmail.com>
In-Reply-To: <201107311342.12547.chriscool@tuxfamily.org>

Hi,

Christian Couder writes:
> On Thursday 28 July 2011 18:52:24 Ramkumar Ramachandra wrote:
>> +     if (opts->mainline) {
>> +             strbuf_reset(&buf);
>
> It is not necessary to reset &buf here.
>
>> +             strbuf_addf(&buf, "%d", opts->mainline);
>> +             git_config_set_in_file(opts_file, "options.mainline", buf.buf);
>> +     }
>
> And perhaps it would be clearer if it was:
>
> +       if (opts->mainline) {
> +               struct strbuf buf = STRBUF_INIT;
> +               strbuf_addf(&buf, "%d", opts->mainline);
> +               git_config_set_in_file(opts_file, "options.mainline", buf.buf);
> +               strbuf_release(&buf);
> +       }

Much clearer.  Fixed.

>> +     if (opts->strategy)
>> +             git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
>> +     if (opts->xopts) {
>
> Other nit: maybe you could put "int i" here, instead of at the beginning of
> the function.

Fixed.  Thanks.

-- Ram