Re: [PATCH v5 2/4] git-cherry-pick: Add keep-redundant-commits option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 16, 2012 at 11:38:27AM -0400, Neil Horman wrote:
> > 
> > I find it a bit strange, that if we cherry-pick a commit that was
> > already empty, we _do_ call git commit (and error out), but if we find a
> > commit that is made empty, we do _not_ call git commit and quietly
> > succeed (in not doing anything). But I suppose that is the legacy
> > behavior?
>
> Correct, more or less.  The legacy behavior is to call git commit unilaterally.
> [...] The only change is that if we do not specify
> keep-redundant-commits, we check to see if the commit is made empty in
> git-cherry-pick and skip it if it is.  We could, instead of returning prior to
> calling git-commit, use that test to override the keep_empty option below, so
> that we don't pass --allow-empty to git-commit instead.  That would preserve the
> prior code path, but for no real advatage, as the outcome is the same, and this
> way saves us having to fork the git-commit command, which I think is
> adventageous.

Except that the outcome is not the same. With and without your changes,
git cherry-pick <empty-commit> fails. But with your changes, git
cherry-pick <commit-will-become-empty> will succeed and do nothing,
while before it would have failed exactly like git cherry-pick
<empty-commit>.

So I am not arguing whether failing or skipping is the better default
behavior. But the legacy behavior is consistent between the empty-commit
and commit-will-become-empty cases.  And if we change the behavior for
one, why not also for the other?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]