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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 26, 2019, 23:07 UTC
Message-ID: <xmqqlfzwtlft.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <cover.1556305561.git.liu.denton@gmail.com>

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

Show 14 quoted lines

> 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.