Re: [PATCH 5/8] rebase: preserve interactive todo file on checkout failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 23, 2023 at 01:16:47PM -0700, Junio C Hamano wrote:
Oswald Buddenhagen <oswald.buddenhagen@xxxxxx> writes:

Creating a suitable todo file is a potentially labor-intensive process,
so be less cavalier about discarding it when something goes wrong (e.g.,
the user messed with the repo while editing the todo).

Is there a reason why we do not always keep it?  Why is the file
sometimes precious but not precious at all in other times?

the unedited initial todo just isn't precious. that implies that in a non-interactive rebase, it is always worthless at the time of the initial reset.

Tying the previous bit to "-i was explicitly given" feels a bit
unintuitive---when the sequencer machinery was implicitly chosen,
and gives the control back to the user, should a user be forbidden
to muck with the todo list?

that would be an --edit-todo and --continue during a mid-rebase stop. rather different case.

No // comments, please.

(apparently with a special exception for examples in the apidocs, presumably because escaping nested comments would be just too ugly.)

-	test_path_is_missing .git/rebase-merge &&
+	test_path_is_dir .git/rebase-merge &&

Are we happy to just see that the directory still exists?  I thought
the original motivation explained in the proposed log message was to
keep the todo list file, so shouldn't you be checking if the file is
there

fair point.

(and if you can reliably ensure that the file has contents
that are expected, that would be even better)?

i could grep for a shortened sha1 i would obtain from the branch. but given that the error scenario of a present but somehow corrupted todo seems implausible given the circumstances, that seems like overkill.

Also, as the keeping of the todo list is now conditional, we should
have another test that checks that the file is gone when that
condition ("INTERACTIVE_EXPLICIT"?) does not trigger, I think.

that would be for t3400-rebase.sh.
i suppose we could extend 'Show verbose error when HEAD could not be detached'.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux