On 2016-11-04 at 17:30:12 +0100, Jeff King <peff@xxxxxxxx> wrote:
> On Fri, Nov 04, 2016 at 04:19:49PM +0100, Tobias Klauser wrote:
>
> > The name parameter to create_branch() has been unused since commit
> > 55c4a673070f ("Prevent force-updating of the current branch"). Remove
> > the parameter and adjust the callers accordingly. Also remove the
> > parameter from the function's documentation comment.
>
> This seemed eerily familiar, and it turns out I wrote this as a
> preparatory step for a different topic a while back, but never finished
> it.
>
> So clearly a good change, though we might want to explain a bit more why
> it's correct that the parameter is unused. Here's what I wrote:
>
> This function used to have the caller pass in the current value of
> HEAD, in order to make sure we didn't clobber HEAD. In 55c4a6730,
> that logic moved to validate_new_branchname(), which just resolves
> HEAD itself. The parameter to create_branch is now unused.
Ah, I didn't know about the history of this parameter. It clearly makes
sense to explain this in the patch description.
> I also ended up reformatting the documentation comment, but that's
> purely optional. My patch is below for reference. Feel free to grab any
> bits of it that you agree with.
I like your documentation comment much better as IMO it's easier to read
and to identify the individual parameters.
Given these facts, I guess it's better if my patch is dropped and yours
is applied instead :)
Thanks a lot!