Re: [PATCH 04/11] reset_head(): remove action parameter

Hi Junio

On 01/10/2021 21:58, Junio C Hamano wrote:

"Phillip Wood via GitGitGadget" [off-list ref] writes:

quoted

From: Phillip Wood <redacted>

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.