Re: [PATCH 1/8] sequencer: introduce new commands to reset the revision

From: Philip Oakley <philipoakley@iee.org>
Date: Jan 18, 2018, 21:24 UTC
Message-ID: <F2A95961E128479484699EC5DDC9243D@PhilipOakley>
In-Reply-To: <CA+P7+xozcQD2xuys6mh8MsfcYZ_nb2c9yxhDxkc7FTN2SfoofQ@mail.gmail.com>

From: "Jacob Keller" <jacob.keller@gmail.com>

Show 22 quoted lines

> On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
>> This commit implements the commands to label, and to reset to, given
>> revisions. The syntax is:
>>
>>         label <name>
>>         reset <name>
>>
>> As a convenience shortcut, also to improve readability of the generated
>> todo list, a third command is introduced: bud. It simply resets to the
>> "onto" revision, i.e. the commit onto which we currently rebase.
>>
>
> The code looks good, but I'm a little wary of adding bud which
> hard-codes a specific label. I suppose it does grant a bit of
> readability to the resulting script... ? It doesn't seem that
> important compared to use using "reset onto"? At least when
> documenting this it should be made clear that the "onto" label is
> special.
>
> Thanks,
> Jake.

I'd agree.

The special 'onto' label should be fully documented, and the commit message should indicate which patch actually defines it (and all its corner cases and fall backs if --onto isn't explicitly given..)

Likewise the choice of 'bud' should be explained with some nice phraseology indicating that we are growing the new flowering from the bud, otherwise the word is a bit too short and sudden for easy explanation.

Philip