On 2009-02-11 10:48:00 +0000, Catalin Marinas wrote:
> 2009/2/11 Karl Hasselström <kha@xxxxxxxxxxx>:
>
> > On 2009-02-10 14:14:07 +0000, Catalin Marinas wrote:
> >
> > > I'm still not entirely sure where the check for stgit.autoimerge
> > > should be done. In the classic infrastructure, it is done in the
> > > merge function. With this patch, it is done in
> > > Transaction.push(). Should we push this even further to
> > > stgit.commands.push? My opinion is not since by having it in
> > > Transaction we get the advantage not listing the conflicts if
> > > the mergetool succeeds and we don't need to abort the
> > > transaction.
> >
> > Yes, one advantage of having it here is that if the user resolves
> > the conflict, we can just continue. I'm not sure I personally like
> > that mode of operation -- you might have guessed by now that I
> > like noninteractive mechanisms -- but I can see how it's useful to
> > someone who does.
>
> I find it useful when I prepare a kernel release and pick patches
> from many branches, it saves some typing with having to run the
> mergetool and restart the pick or push command. It's also useful for
> "sync".
>
> > Another advantage of having it here is that it automatically just
> > works for all commands, not just "stg push".
>
> It works for commands that use Transaction.push_patch(). Other
> commands that use IndexAndWorktree.merge() via some other function
> would not work. Will there be such functions? I suspect a "sync"
> implementation would need additional support in Transaction.
Yes, you're right.
> Any thoughts on calling mergetool from IndexAndWorktree.merge()
> (with an additional parameter to explicitly enable this rather than
> just reading the config option)?
That could very well be a good idea -- I can't think of anything wrong
with it. (And it's a good idea to make this a parameter rather than
making it read the config option.)
> > The disadvantage that I see is that we ask the user to put work
> > into resolving conflicts before we've made sure that we won't roll
> > back the whole transaction. If this is to become a dependable
> > feature, we need a way to make sure we'll never throw away the
> > user's work.
>
> Maybe push_patch() can receive a parameter on whether to invoke
> mergetool. The calling code should know the behaviour for aborting
> transactions and only ask for interactivity if the command is
> expected to leave conflicts in the index.
That sounds like a plan.
--
Karl Hasselström, kha@xxxxxxxxxxx
www.treskal.com/kalle
--
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
