Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()

From: Paul Tan <pyokagan@gmail.com>
Date: Mar 16, 2016, 12:28 UTC
Message-ID: <CACRoPnS4JpNNACz4T0F0vFs3ogG+nzk-y1=zc1UrtAZKaEnggg@mail.gmail.com>
In-Reply-To: <alpine.DEB.2.20.1603160901520.4690@virtualbox>

Hi Dscho,

On Wed, Mar 16, 2016 at 4:04 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 6 quoted lines

> In addition I want to point out that sequencer's replay_opts seem to be at
> least related, but the patch shares none of its code with the sequencer.
> Let's avoid that.
>
> In other words, let's try to add as little code as possible when we can
> enhance existing code.

Well, both git-rebase--am.sh and git-rebase--merge.sh do not use the sequencer functionality at all, and we don't see git-am for example needing to be aware of onto, orig-head, head-name etc.

Besides, I don't see why the sequencer needs to be aware of these rebase-specific options. For simplicity[1], I would think the sequencer would only need to be aware of what the todo list is, since that is common to cherry-pick/revert and rebase-i, and all the other non-sequencer related stuff like checking out the --onto <newbase>, updating refs can be done at the rebase-interactive.c or git-rebase--interactive.sh layer.

[1] Of course, it's kind of unfortunate that sequencer.c has to be aware of whether it is being called as cherry-pick or revert, but I don't see why implementing interactive rebase functionality needs to make the same mistake.

Thanks, Paul