From: Tobias Klauser Date: Fri, 04 Nov 2016 16:52:43 GMT Subject: Re: [PATCH] branch: remove unused parameter to create_branch() Message-ID: <20161104165236.GC819@distanz.ch> In-Reply-To: <20161104163012.5r3uivnub3bdkqgr@sigill.intra.peff.net> On 2016-11-04 at 17:30:12 +0100, Jeff King 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!