Re: [PATCH 1/1] sequencer: fix empty commit check when amending

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Junio

On 22/11/2019 06:40, Junio C Hamano wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> This fixes a regression introduced in 356ee4659b ("sequencer: try to
>> commit without forking 'git commit'", 2017-11-24). When amending a
>> commit try_to_commit() was using the wrong parent when checking if the
>> commit would be empty. When amending we need to check against HEAD^ not
>> HEAD.
> 
> Thanks.  That makes sense.
> 
> If you compare with HEAD and find there is no difference, that would
> mean that the "amend" itself is a no-op (i.e. the user said they
> want to make changes, but after all changed their mind)?  It might
> be something worth noticing, but certainly not in the same way as
> "we were told to skip an empty commit---is this one empty?" gets
> treated.

Yes I'm trying to decide what to do about fixups and squashes that 
become empty. We'd need to do such a check in the sequencer before 
trying to commit I think.

Another thing to think about for the future is what message we want to 
display when a fixup makes the amended commit become empty. If there is 
another fixup following it then the commit may not end up empty by the 
time we've finished applying all the fixups for that commit. If the user 
runs 'reset HEAD^' after the first fixup we'll end up fixing up the 
wrong commit with the later fixups.

Best Wishes

Phillip