Re: [RFC PATCH 0/9] rebase -i: extend rebase.missingCommitsCheck to `--edit-todo' and co.
- From
Alban Gruin <alban.gruin@gmail.com>
- Date
- Jul 25, 2019, 20:26 UTC
- Message-ID
- <69ceed35-ced4-51e7-d724-1818ae45e9a6@gmail.com>
- In-Reply-To
- <ce6bdffd-0a04-803c-4a1e-0959342db01f@gmail.com>
Hi Phillip,
Le 24/07/2019 à 15:29, Phillip Wood a écrit :
Show 21 quoted lines
> Hi Alban > > Thanks for working on this, it's great to see you back on the list and I > think it would be a useful addition to rebase. Unfortunately I'm not > sure about this implementation though (although the early bug fix > patches are useful in their own right) > > On 17/07/2019 15:39, Alban Gruin wrote: >> To prevent mistakes when editing a branch, rebase features a knob, >> rebase.missingCommitsCheck, to warn the user if a commit was dropped. >> Unfortunately, this check is only effective for the initial edit, which >> means that if you edit the todo list at a later point of the rebase and >> dropped a commit, no warnings or errors would be issued. >> >> This adds the ability to check if commits were dropped when resuming a >> rebase (with `rebase --continue'), when editing the todo list (with >> `rebase --edit-todo'), or when reloading the todo list after an `exec' >> command. > > I'm not sure if we really need to check the todo list when continuing or > after an exec command.
In which case I don’t really understand why there is an `error' mode if one can completely bypass it with `--continue'.
Show 10 quoted lines
> The official way to edit the todo list is to run > 'git rebase --edit-todo' and I'm not sure if we support scripts writing > to .git/rebase-merge/git-rebase-todo directly. If we only support the > check after --edit-todo then I think the implementation can be > simplified as we can just write a copy of the file before it is edited > and don't need to check .git/rebase-merge/done. Additionally that would > catch commits that are added by the user and then deleted in a later > edit. They wont be in the original list so I don't think this series > will detect their deletion. >
True -- but with this solution, if a bad command is introduced, there will be false negatives. Given the pitfall of my solution, this should be an acceptable trade-off.
Show 11 quoted lines
> At the extreme I have a script around rebase that runs 'rebase -i HEAD' > and then fills in the todo list with a fake editor that adds 'reset ...' > as the first line to set the starting point of the rebase. I think > dscho's garden-shears script does something similar. Under the proposed > scheme if I subsequently edit the todo list it will not catch any > deleted commits as the original list is empty. > > Best Wishes > > Phillip >
Cheers, Alban