git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/3] rebase -i: drop, missing commits and static checks

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 30, 2015, 16:16 UTC
Message-ID
<xmqq7fql8949.fsf@gitster.dls.corp.google.com>
In-Reply-To
<1849903896.89424.1435676774069.JavaMail.zimbra@ensimag.grenoble-inp.fr>

Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:

Show 23 quoted lines
> Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:
>> Shouldn't all the checking also be called in a 'rebase --continue',
>> considering that it can be called after a 'rebase --edit-todo' ?
>> (Right now it is only called after closing the editor in 'rebase -i')
>
> What's your opinion on it?
>
> Short example:
>
> 'git rebase -i HEAD~2'
> 	pick commit_sha_1 commit_msg_1
> 	tick commit_sha_2 commit_msg_2
> An error is raised before anything is done.
> 'git rebase --edit-todo'
> 	pick commit_sha_1 commit_msg_1
> 	tick commit_sha_2 commit_msg_2
> (nothing changed)
> 'git rebase --continue'
> An error is raised after having picked the first commit.
>
> The same is relevent with bad sha and missing commits (in fact even
> more relevant with missing commits since that would be silent loss of
> information).

The place where an error can be introduced is (assuming that what "rebase -i" writes out itself is perfect ;-) where we allow the user to edit, so instead of checking before "--continue", I would expect a sane design would check immediately after the editor we spawned returns.

The codepath that allows the insn sheet getting edited and the codepath that handles --edit-todo have to do many things the same way (i.e. use collapse_todo_ids to prepare the file for editing, spawn the editor, receive the result and use expand_todo_ids to prepare the file to be used by us), and I would have expected for these two to be using a single helper---and a sanity check like the above can and should be done when we receive the result from the editor, immediately before running expand_todo_ids perhaps.

If they are not using such a single helper right now, perhaps that is the preliminary restructuring of the code that is needed?

Previous: Matthieu MoyNext: Remi Galan Alfonso
Message 11 of 14 in “rebase -i: drop, missing commits and static checks”
  1. Galan RémiJun 29, 2015
  2. 1/3 git-rebase -i: add command "drop" to remove a commitGalan Rémi, Jun 29, 2015
  3. 2/3 git rebase -i: warn about removed commitsGalan Rémi, Jun 29, 2015
  4. 3/3 git rebase -i: add static check for commands and SHA-1Galan Rémi, Jun 29, 2015
  5. 0/3 rebase -i: drop, missing commits and static checksMatthieu Moy, Jun 30, 2015
  6. 1/3 fixup! git rebase -i: add static check for commands and SHA-1Matthieu Moy, Jun 30, 2015
  7. 2/3 fixup! git rebase -i: warn about removed commitsMatthieu Moy, Jun 30, 2015
  8. 3/3 fixup! git rebase -i: warn about removed commitsMatthieu Moy, Jun 30, 2015
  9. Remi Galan AlfonsoJun 30, 2015
  10. Matthieu MoyJun 30, 2015
  11. Junio C HamanoJun 30, 2015
  12. Remi Galan AlfonsoJun 30, 2015
  13. Matthieu MoyJun 30, 2015
  14. Junio C HamanoJun 30, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.