From: Phillip Wood Date: Mon, 04 Oct 2021 10:00:22 GMT Subject: Re: [PATCH 04/11] reset_head(): remove action parameter Message-ID: <97b8cbdb-60f9-cba5-2db0-6ae77a9afd60@gmail.com> In-Reply-To: Hi Junio On 01/10/2021 21:58, Junio C Hamano wrote: > "Phillip Wood via GitGitGadget" writes: > >> From: Phillip Wood >> >> The action parameter is passed as the command name to >> setup_unpack_trees_porcelain(). All but two cases pass either >> "checkout" or "reset". The case that passes "reset --hard" should be >> passing "reset" instead. > > Describe how the parameter is meant to be used (presumably "this is > to record in the reflog", perhaps?); without such explanation, it is > hard to either agree or disagree with the claim that "reset --hard" > should be "reset". How about The only use of the action parameter is to setup the error messages for unpack_trees(). All but two cases pass either "checkout" or "reset". The case that passes "reset --hard" would be better passing "reset" so that the error messages match the builtin reset command like all the other callers that are doing a reset. The case that passes "Fast-forwarded" is only updating HEAD and so the parameter is unused in that case as it does not call unpack_trees(). The value to pass to setup_unpack_trees_porcelain() can be determined by checking whether flags contains RESET_HEAD_HARD without the caller having to specify it. > Also state if this change is supposed to have any externally > observable effect. It'll change the error message if we cannot clear the stashed changes form the working tree from saying "reset --hard" to "reset". Best Wishes Phillip > Perhaps this improves what is shown in an error message by affecting > what setup_unpack_trees_porcelain() does? I am just guessing, > because the proposed log message is not telling. Please do not make > me (or other readers of "git log") guess. > > Thanks. >