"Catalin Marinas" <catalin.marinas@xxxxxxxxx> writes: > On 08/10/2007, Karl Hasselström <kha@xxxxxxxxxxx> wrote: >> On 2007-10-08 14:16:10 +0100, Catalin Marinas wrote: >> >> > On 14/09/2007, David Kågedal <davidk@xxxxxxxxxxxxxx> wrote: >> > >> > > + assert commit or (top and bottom) >> > > + assert not before_existing or (top and bottom) >> > > + assert not (commit and before_existing) >> > > + assert (top and bottom) or (not top and not bottom) >> > > + assert not top or (bottom == git.get_commit(top).get_parent()) >> > >> > The last assertion here prevents the use of 'stg pick --reverse'. >> > This command creates an unapplied patch with top and bottom reversed >> > and pushes it to force a three-way merge. >> > >> > It seems to work OK if I comment it out but I wonder whether it will >> > break in the future with the planned removal of the top and bottom >> > files. >> >> I think the assert represents a real constraint, namely that there has >> to be a 1:1 correspondance between patches and commits. Yes, that was the point of the series. >> Couldn't "stg pick --reverse" create a new commit and use that? That >> is, given that we want to revert commit C, create a new commit C* with > > Series.new_patch already creates a commit, why should we move the > functionality to 'pick'? The only call to new_patch with commit=False > seems to be from 'uncommit' (and it makes sense indeed). It might be true that the assertion could be amended so that if commit=True, then it is allowed to have top/bottom that doesn't correspond to a commit and its parent. But it's hard to be sure without looking at the code much harder than I did just now. Testing would also help. >> And shouldn't there be a test for this? :-) > > Yes :-). I think there are many other tests needed. It would be useful > to do a code coverage with the existing tests to see what we are > missing. Unit testing might be useful as well but we all have limited > spare time. -- David Kågedal - 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
