Re: [PATCH] branch: remove unused parameter to create_branch()

On 2016-11-04 at 17:30:12 +0100, Jeff King [off-list ref] wrote:

On Fri, Nov 04, 2016 at 04:19:49PM +0100, Tobias Klauser wrote:

quoted

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!