Hi Phillip, Le 24/07/2019 à 15:29, Phillip Wood a écrit : > 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'. > 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. > 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
