From: Phillip Wood Date: Wed, 09 Aug 2023 10:06:36 GMT Subject: Re: [PATCH v3 0/7] rebase -i: impove handling of failed commands Message-ID: In-Reply-To: On 07/08/2023 21:16, Glen Choo wrote: > Hi Phillip! > > "Phillip Wood via GitGitGadget" writes: > >> This series fixes several bugs in the way we handle a commit cannot be >> picked because it would overwrite an untracked file. >> >> * after a failed pick "git rebase --continue" will happily commit any >> staged changes even though no commit was picked. >> >> * the commit of the failed pick is recorded as rewritten even though no >> commit was picked. >> >> * the "done" file used by "git status" to show the recently executed >> commands contains an incorrect entry. >> >> Thanks to Eric, Glen and Junio for their comments on v2. Here are the >> changes since v2: > > Thanks for sending this version, and apologies for not getting to it > sooner (I tried a few times, but it was hard to reconstruct the context > around something as complicated as sequencer.c..). Unfortunately, I > don't think I will be able to chime in on subsequent rounds. Thanks again for you comments on the last round, they were really helpful in improving the commit messages. >> Patch 1 - Reworded the commit message. >> >> Patch 2 - Reworded the commit message, added a test and fixed error message >> pointed out by Glen. >> >> Patch 3 - New cleanup. >> >> Patch 4 - Reworded the commit message, now only increments >> todo_list->current if there is no error. >> >> Patch 5 - Swapped with next patch. Reworded the commit message, stopped >> testing implementation (suggested by Glen). Expanded post-rewrite hook test. >> >> Patch 6 - Reworded the commit message, now uses the message file rather than >> the author script to check if "rebase --continue" should commit staged >> changes. Junio suggested using a separate file for this but I think that >> would end up being more involved as we'd need to be careful about creating >> and removing it. >> >> Patch 7 - Reworded the commit message. > > I found the updated commit messages much easier to understand, and the > change to no longer test implementation is also very welcome, so > overall, I think this is a marked improvement over the previous version. Thanks, I'm glad the messages are easier to understand now > Like Junio, I'm not familiar enough with sequencer or its 'expected > behavior' to feel comfortable LGTM-ing the later patches. Yes, I'm hoping Dscho will have time to take a look at them once 2.42.0 is out. Best Wishes Phillip