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

Re: [PATCH/RFCv2 1/2] git-rebase -i: add command "drop" to remove a commit

From
Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr>
Date
Jun 3, 2015, 09:13 UTC
Message-ID
<787870724.57987.1433322837954.JavaMail.zimbra@ensimag.grenoble-inp.fr>
In-Reply-To
<xmqqk2vm3udy.fsf@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> As long as what is given to 'drop' 
> is checked when it matters (e.g. when the code in patch 2/2 tries 
> see if some commits in the original list are no longer there in 
> order to warn sees "drop foo bar" where "foo" is obviously not an 
> object name in the original list, that should be checked), it is 
> fine. And I agree 1/2 is not the place to do so, even though it may 
> be easier from the implementation point of view (which is why I 
> mentioned the possibility in the review of that patch).

I disagree, I think that that either the checking for the 'drop' command should either be in the 1/2 where it is introduced or in the function check_commits introduced by 2/2 but in a separate commit/patch. The 2/2 checks if there are removed commits to have the possibility to avoid silent loss of information. It is not its role to check if the SHA-1 following 'drop' are correct.

What I think should be the best for now is checking the SHA-1 following 'drop' in 1/2 (or not checking at all) and change the whole checking system of rebase in a later patch (e.g. check beforehand (probably in check_commits) if the commands exist, if the SHA-1 are correct and possibly other things). 

Rémi
Previous: Junio C HamanoNext: Junio C Hamano
Message 10 of 12 in “rebase -i : drop command and removed commits”
  1. 0/2 rebase -i : drop command and removed commitsGalan Rémi, Jun 1, 2015
  2. 1/2 git-rebase -i: add command "drop" to remove a commitGalan Rémi, Jun 1, 2015
  3. Junio C HamanoJun 1, 2015
  4. Matthieu MoyJun 1, 2015
  5. Remi Galan AlfonsoJun 1, 2015
  6. Matthieu MoyJun 1, 2015
  7. Remi Galan AlfonsoJun 2, 2015
  8. Matthieu MoyJun 2, 2015
  9. Junio C HamanoJun 2, 2015
  10. Remi Galan AlfonsoJun 3, 2015
  11. Junio C HamanoJun 3, 2015
  12. 2/2 git rebase -i: warn about removed commitsGalan Rémi, Jun 1, 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.