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

From: Tay Ray Chuan <rctay89@gmail.com>
To: Erick Mattos <erick.mattos@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Clemens Buchacher <drizzd@aon.at>,
	Michel Lespinasse <walken@google.com>
Subject: Re: [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to  a OPTION_SET_INT
Date: Thu, 24 Jun 2010 03:04:41 +0800	[thread overview]
Message-ID: <AANLkTikYgbVlyLlaba2G2CTFc0wFgylPRvhm-BX0igFz@mail.gmail.com> (raw)
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:
> 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