From: Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr>
Date: Wed, 03 Jun 2015 09:13:57 GMT
Subject: Re: [PATCH/RFCv2 1/2] git-rebase -i: add command "drop" to remove a commit
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: 
> 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