All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
	Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
	Philippe Blain <levraiphilippeblain@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Elijah Newren <newren@gmail.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH 04/11] reset_head(): remove action parameter
Date: Mon, 4 Oct 2021 11:00:22 +0100	[thread overview]
Message-ID: <97b8cbdb-60f9-cba5-2db0-6ae77a9afd60@gmail.com> (raw)
In-Reply-To: <xmqqpmso5uno.fsf@gitster.g>

Hi Junio

On 01/10/2021 21:58, Junio C Hamano wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> 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.
> 

  reply	other threads:[~2021-10-04 10:00 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-01 10:04 [PATCH 00/11] rebase: reset_head() related fixes and improvements Phillip Wood via GitGitGadget
2021-10-01 10:04 ` [PATCH 01/11] rebase: factor out checkout for up to date branch Phillip Wood via GitGitGadget
2021-10-01 10:04 ` [PATCH 02/11] reset_head(): fix checkout Phillip Wood via GitGitGadget
2021-10-01 20:26   ` Junio C Hamano
2021-10-04  9:58     ` Phillip Wood
2021-10-04 16:13       ` Junio C Hamano
2021-10-01 22:47   ` Eric Sunshine
2021-10-01 10:04 ` [PATCH 03/11] reset_head(): don't run checkout hook if there is an error Phillip Wood via GitGitGadget
2021-10-01 20:52   ` Junio C Hamano
2021-10-04 10:00     ` Phillip Wood
2021-10-12  8:48       ` Ævar Arnfjörð Bjarmason
2021-10-01 10:04 ` [PATCH 04/11] reset_head(): remove action parameter Phillip Wood via GitGitGadget
2021-10-01 20:58   ` Junio C Hamano
2021-10-04 10:00     ` Phillip Wood [this message]
2021-10-01 10:04 ` [PATCH 05/11] reset_head(): factor out ref updates Phillip Wood via GitGitGadget
2021-10-01 21:00   ` Junio C Hamano
2021-10-04 10:03     ` Phillip Wood
2021-10-01 10:04 ` [PATCH 06/11] reset_head(): make default_reflog_action optional Phillip Wood via GitGitGadget
2021-10-01 21:03   ` Junio C Hamano
2021-10-01 21:08   ` Junio C Hamano
2021-10-04 10:03     ` Phillip Wood
2021-10-01 10:04 ` [PATCH 07/11] rebase: cleanup reset_head() calls Phillip Wood via GitGitGadget
2021-10-01 10:04 ` [PATCH 08/11] reset_head(): take struct rebase_head_opts Phillip Wood via GitGitGadget
2021-10-01 21:11   ` Junio C Hamano
2021-10-04 10:09     ` Phillip Wood
2021-10-01 10:05 ` [PATCH 09/11] rebase --apply: fix reflog Phillip Wood via GitGitGadget
2021-10-01 21:12   ` Junio C Hamano
2021-10-01 10:05 ` [PATCH 10/11] rebase --apply: set ORIG_HEAD correctly Phillip Wood via GitGitGadget
2021-10-01 21:18   ` Junio C Hamano
2021-10-01 10:05 ` [PATCH 11/11] rebase -m: don't fork git checkout Phillip Wood via GitGitGadget
2021-10-02  0:38 ` [PATCH 00/11] rebase: reset_head() related fixes and improvements Junio C Hamano
2021-10-02  4:58   ` Junio C Hamano
2021-10-02 12:27     ` Phillip Wood
2021-10-02 13:12       ` Phillip Wood
2021-10-02 13:38       ` René Scharfe
2021-10-06 14:03         ` Phillip Wood
2021-12-08 14:57 ` [PATCH v2 00/14] " Phillip Wood via GitGitGadget
2021-12-08 14:57   ` [PATCH v2 01/14] rebase: factor out checkout for up to date branch Phillip Wood via GitGitGadget
2021-12-09 21:04     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 02/14] t5403: refactor rebase post-checkout hook tests Phillip Wood via GitGitGadget
2021-12-09 18:24     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 03/14] rebase: pass correct arguments to post-checkout hook Phillip Wood via GitGitGadget
2021-12-09 18:53     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 04/14] rebase: do not remove untracked files on checkout Phillip Wood via GitGitGadget
2021-12-09 19:09     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 05/14] rebase --apply: don't run post-checkout hook if there is an error Phillip Wood via GitGitGadget
2021-12-08 14:57   ` [PATCH v2 06/14] reset_head(): remove action parameter Phillip Wood via GitGitGadget
2021-12-08 14:57   ` [PATCH v2 07/14] create_autostash(): remove unneeded parameter Phillip Wood via GitGitGadget
2021-12-09 19:17     ` Junio C Hamano
2022-01-25 11:06       ` Phillip Wood
2021-12-08 14:57   ` [PATCH v2 08/14] reset_head(): factor out ref updates Phillip Wood via GitGitGadget
2021-12-08 14:57   ` [PATCH v2 09/14] reset_head(): make default_reflog_action optional Phillip Wood via GitGitGadget
2021-12-09 19:23     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 10/14] rebase: cleanup reset_head() calls Phillip Wood via GitGitGadget
2021-12-09 19:26     ` Junio C Hamano
2022-01-25 11:07       ` Phillip Wood
2021-12-08 14:57   ` [PATCH v2 11/14] reset_head(): take struct rebase_head_opts Phillip Wood via GitGitGadget
2021-12-09 19:31     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 12/14] rebase --apply: fix reflog Phillip Wood via GitGitGadget
2021-12-09 20:49     ` Junio C Hamano
2021-12-08 14:58   ` [PATCH v2 13/14] rebase --apply: set ORIG_HEAD correctly Phillip Wood via GitGitGadget
2021-12-11 10:59     ` Elijah Newren
2021-12-08 14:58   ` [PATCH v2 14/14] rebase -m: don't fork git checkout Phillip Wood via GitGitGadget
2021-12-09 21:04   ` [PATCH v2 00/14] rebase: reset_head() related fixes and improvements Junio C Hamano
2022-01-26 10:53     ` Phillip Wood
2022-01-27 17:37       ` Junio C Hamano
2021-12-11 11:05   ` Elijah Newren
2022-01-26 13:05   ` [PATCH v3 " Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 01/14] rebase: factor out checkout for up to date branch Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 02/14] t5403: refactor rebase post-checkout hook tests Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 03/14] rebase: pass correct arguments to post-checkout hook Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 04/14] rebase: do not remove untracked files on checkout Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 05/14] rebase --apply: don't run post-checkout hook if there is an error Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 06/14] reset_head(): remove action parameter Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 07/14] reset_head(): factor out ref updates Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 08/14] reset_head(): make default_reflog_action optional Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 09/14] create_autostash(): remove unneeded parameter Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 10/14] rebase: cleanup reset_head() calls Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 11/14] reset_head(): take struct rebase_head_opts Phillip Wood via GitGitGadget
2022-01-26 13:35       ` Ævar Arnfjörð Bjarmason
2022-01-26 14:52         ` Phillip Wood
2022-01-26 13:05     ` [PATCH v3 12/14] rebase --apply: fix reflog Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 13/14] rebase --apply: set ORIG_HEAD correctly Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 14/14] rebase -m: don't fork git checkout Phillip Wood via GitGitGadget
2022-02-01 17:03     ` [PATCH v3 00/14] rebase: reset_head() related fixes and improvements Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=97b8cbdb-60f9-cba5-2db0-6ae77a9afd60@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=levraiphilippeblain@gmail.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.