From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Mon, 13 Nov 2017 03:06:53 GMT
Subject: Re: [RFC PATCH v3 2/4] branch: re-order function arguments to group related arguments
Message-ID: <1510542413.5134.2.camel@gmail.com>
In-Reply-To: <xmqqefp2c3i0.fsf@gitster.mtv.corp.google.com>

On Mon, 2017-11-13 at 11:32 +0900, Junio C Hamano wrote:
> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
> 
> > I've tried to improve it, does the following paragraph sound clear
> > enough?
> > 
> >     branch: group related arguments of create_branch()
> >         
> >     New arguments were added to create_branch() whenever the need
> >     arised and they were added to tail of the argument list. This
> >     resulted in the related arguments not being close to each other.
> 
> OK, I understand what you wanted to say.  But I do not think that is
> based on a true history.
> 
>  - f9a482e6 ("checkout: suppress tracking message with "-q"",
>    2012-03-26) adds 'quiet' just after 'clobber_head', exactly
>    because they are related, and leaves 'track' at the end.
> 
>  - 39bd6f72 ("Allow checkout -B <current-branch> to update the
>    current branch", 2011-11-26) adds 'clobber_head' not at the end but
>    before 'track', which is left at the end.  
> 
>  - c847f537 ("Detached HEAD (experimental)", 2007-01-01) split 'start'
>    into 'start_name' and 'start_sha1' (the latter was laster removed)
>    and this was not a mindless "add at the end", either.
> 
>  - 0746d19a ("git-branch, git-checkout: autosetup for remote branch
>    tracking", 2007-03-08) did add track at the end, but that is
>    justifiable, as it has no relation to any other parameter.
> 

Seems I wasn't careful enough in noticing how the arguments were added.
I seemed to have overlooked the fact that 39bd6f72 added 'clobber_head'
"before" track which resulted in the vague commit message. Anyways,
thanks for taking the time to dig into this.


> You could call 39bd6f72 somewhat questionable as 'clobber_head' is
> related to 'force' more strongly than it is to 'reflog' [*1*], but
> it is unfair to blame anything else having done a mindless "add at
> the end".
> 

Yep, you're right. How does the following sound?

    branch: group related arguments of create_branch()
    
    39bd6f726 (Allow checkout -B <current-branch> to update the current
    branch, 2011-11-26) added 'clobber_head' (now, 'clobber_head_ok')
    "before" 'track' as 'track' was closely related 'clobber_head' for
    the purpose the commit wanted to achieve. Looking from the perspective
    of how the arguments are used it turns out that 'clobber_head' is
    more related to 'force' than it is to 'track'.
    
    So, re-order the arguments to keep the related arguments close
    to each other.
    
-- 
Kaartic