Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base

[Junio C Hamano <gitster@pobox.com>]

Denton Liu <liu.denton@gmail.com> writes:

> Thanks for your comments, Eric and Junio.
>
> Eric, I've combined the `test_when_finished` calls together so that the
> statements within appear in a more "logical" order.
>
> Junio, I've taken your suggestion and moved the change into
> `create_branch`. Initially, I didn't want to do this because I didn't
> want to change the semantics of git-branch but introducing the merge
> base syntax seems to be a positive change so let's do it.
> ...
> Denton Liu (3):
>   t2018: cleanup in current test
>   t2018: demonstrate checkout -b merge base bug
>   branch: make create_branch accept a merge base rev

Because "checkout -b new" is supposed to be merely a short-hand for
a "branch new" followed by "checkout new", the lack of "branch new
A...B" is the same "bug" as the lack of "checkout -b new A...B".

The second patch that does not talk about the former but singles out
only the latter is being inconsistent.

One person's lack of feature is a bug to another person, and indeed,
when we did "checkout A...B" in 2009, we weren't interested in doing
the same for "checkout -b new", and nobody thought about adding that
until now, and/or considered the lack of feature as a bug.

We do not "demonstrate" the lack of a new feature in a patch with
expect-failure, followed by another patch that adds the feature that
flips expect-failure to expect-success.  A patch that teaches
"checkout -b" about A...B, that is adding a missing feature, should
not have to do so.  As it is shades of gray between a change being a
bugfix and adding a new feature, switching the style of testing
based on the distinction between them does not make much sense.  Be
consistent and stick to just one style.  And having the test and the
code change (be it adding a missing feature or fixing a bug) in a
single patch makes patch management a lot simpler by making it
harder to lose only one half.

Having a preliminary clean-up as a separate step is a good idea, but
for this topic, I think the latter two should be combined into a
single patch that changes the code and adds tests at the same time.

Thanks.