Re: [PATCH 14/18] revert: Introduce --reset to remove sequencer state
- From
Ramkumar Ramachandra <artagnon@gmail.com>
- Date
- Jul 27, 2011, 14:35 UTC
- Message-ID
- <CALkWK0nKMMjT1jg8zENmwbxvFU+5A+BFLVkoDRAOtURNcZ4H7A@mail.gmail.com>
- In-Reply-To
- <20110727142815.GC24785@elie>
Hi,
Jonathan Nieder writes:
Show 31 quoted lines
> Ramkumar Ramachandra wrote:
>> Jonathan Nieder writes:
>>> Ramkumar Ramachandra wrote:
>
>>>> + if (opts->subcommand == REPLAY_RESET) {
>>>> + remove_sequencer_state(1);
>>>> + return 0;
>>>> + } else {
>>>> + /* Start a new cherry-pick/ revert sequence */
>>>
>>> Can un-indent by dropping the "else":
>>
>> Actually this was intentional; if we un-indent this now, there'll be a
>> diff indenting it when '--reset' and '--continue' are introduced which
>> turns out to be especially ugly :)
>
> Why couldn't it look like this at the end, for example? (As always,
> this is just an example; I am not saying "please make it look like
> this".)
>
> if (opts->subcommand == REPLAY_RESET) {
> remove_sequencer_state(1);
> return 0;
> }
>
> if (opts->subcommand == REPLAY_CONTINUE) {
> ... prepare todo list for continue ...
> } else {
> ... prepare todo list for a new cherry-pick ...
> }
> pick the chosen commitsMainly style. It looks a little unnatural: when more subcommands come up, I'd like a nice switch-case to dispatch :)
-- Ram