Re: [PATCH v5 3/4] git-cherry-pick: Add test to validate new options

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

 



On Tue, Apr 17, 2012 at 11:38:52PM +0200, Clemens Buchacher wrote:
> On Tue, Apr 17, 2012 at 06:56:04AM -0400, Neil Horman wrote:
> > On Mon, Apr 16, 2012 at 11:42:49PM +0200, Clemens Buchacher wrote:
> > > 
> > > It seems that I was implying a lot more than I realized. What I meant
> > > was that master and empty-branch2 are equivalent for the purposes of
> > > that test (empty-branch2^ also is a non-empty commit [*1*]), but while
> > > master is a moving target, empty-branch2 is untouched. 
> > > 
> > for the purposes of the --keep-redundant-commits however, the target is
> > irrelevant.  The only requirement is that we cherry-pick a commit that is
> > guaranteed to become empty when applied.
> 
> That we agree on.
> 
> > We certainly could do that on empty branch2, but theres no advantage
> > to doing so,
> 
> The advantage is that I do not have to read the other tests in order to
> understand what this test does, because contrary to the master branch,
> they do not modify empty-branch2.
> 
But empty-branch2, as the name implies, doesn't have a non-empty commit on it
yet, and so theres nothing to cherry-pick on that branch that would 'become'
empty.  We could fix that of course, but I'd worry that that would look odd
against the other tests.

> > and given that every other test attempts to cherry-pick to master, I
> > rather like the consistency.
> 
> We could also consistently not use the master branch.
> 
Yes, This would be my perferred solution I think.  That way we clarify the test
to your satisfaction and keep the consistency of the test methodology.

> > > However, I just notice that empty-branch2 is also the root commit, so
> > > maybe this will not work after all. But that should be easy to fix.
> >
> > It is easy to fix, given your clarified description above, its just that IMO,
> > its not broken.
> 
> Well, I don't mind too badly if this doesn't go may way. But I hope that
> I managed at least to explain my point.
> 
Yes, absolutely.  How about this:  Given that we agree on our ability to
consistently not use master above, what if we table this discussion for now,
leave the tests as they are, and fix them all up in a separate patch set?  I
don't mind making the change your requesting at all, but I'd rather do it as
part of modifying all the tests not to use master, and doing it in a separate
changeset, so we don't convolute what this series is doing.  Does that sound
reasonable?
Neil

--
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]