Re: [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT

From: Tay Ray Chuan <rctay89@gmail.com>
Date: Jun 23, 2010, 19:04 UTC
Message-ID: <AANLkTikYgbVlyLlaba2G2CTFc0wFgylPRvhm-BX0igFz@mail.gmail.com>
In-Reply-To: <AANLkTikyugBxwyCu0GY4dTLW6n_OQhIPNnPfd0_i6k0t@mail.gmail.com>

Hi,

On Tue, Jun 22, 2010 at 7:04 AM, Erick Mattos <erick.mattos@gmail.com> wrote:

Show 23 quoted lines

> Hi,
>
> 2010/6/21 Tay Ray Chuan <rctay89@gmail.com>:
>> diff --git a/builtin/checkout.c b/builtin/checkout.c
>> index 1994be9..e794e1e 100644
>> --- a/builtin/checkout.c
>> +++ b/builtin/checkout.c
>> @@ -32,7 +32,8 @@ struct checkout_opts {
>>        int writeout_stage;
>>        int writeout_error;
>>
>> -       const char *new_branch;
>> +       int new_branch;
>> +       const char *new_branch_name;
>
> The change of name of the existent variable creates more hassle than
> helps.
>
> As you are adding a new option I suggest you to create a new
> variable named new_branch_forced or whatever.  This way you avoid
> making a lot of changes as you did and minimize the possibility of
> adding new bugs by not catching all the problems affected by the
> name change.

Erick, thanks for the suggestion - I've a new iteration that takes some of your points.

-- 
Cheers,
Ray Chuan