From: Denton Liu <email@example.com>
To: Junio C Hamano <firstname.lastname@example.org>
Cc: Git Mailing List <email@example.com>,
Eric Sunshine <firstname.lastname@example.org>
Subject: Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base
Date: Fri, 26 Apr 2019 16:40:24 -0700 [thread overview]
Message-ID: <20190426234024.GA28055@dev-l> (raw)
On Sat, Apr 27, 2019 at 08:07:34AM +0900, Junio C Hamano wrote:
> Denton Liu <email@example.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".
I didn't consider it to be the same "bug" because git-checkout.txt
mentions the "..." syntax, whereas it isn't mentioned in git-branch.txt.
That being said, now that I look at git-checkout.txt again, when doing a
"regular" checkout, the parameter is called <branch> whereas when we're
doing a checkout -b, it's called <start_point>, which doesn't mention
> 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.
I agree, based on the above, I now see that it's a lack of feature and
not 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.
I'm going send a reroll to update the documentation to mention "..." in
<start_point> and, while I'm at it, I'll do the squash.
Thanks for your comments,
next prev parent reply other threads:[~2019-04-26 23:40 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-25 21:10 [PATCH 0/3] checkout: allow -b/-B to work on a merge base Denton Liu
2019-04-25 21:10 ` [PATCH 1/3] t2018: cleanup in current test Denton Liu
2019-04-25 22:34 ` Eric Sunshine
2019-04-26 0:40 ` Denton Liu
2019-04-26 2:50 ` Junio C Hamano
2019-04-26 6:58 ` Eric Sunshine
2019-04-25 21:10 ` [PATCH 2/3] t2018: demonstrate checkout -b merge base bug Denton Liu
2019-04-25 21:10 ` [PATCH 3/3] checkout: allow -b/-B to work on a merge base Denton Liu
2019-04-26 3:02 ` Junio C Hamano
2019-04-26 4:59 ` Junio C Hamano
2019-04-26 19:21 ` [PATCH v2 0/3] allow checkout and branch to create branches " Denton Liu
2019-04-26 19:21 ` [PATCH v2 1/3] t2018: cleanup in current test Denton Liu
2019-04-26 19:21 ` [PATCH v2 2/3] t2018: demonstrate checkout -b merge base bug Denton Liu
2019-04-26 19:21 ` [PATCH v2 3/3] branch: make create_branch accept a merge base rev Denton Liu
2019-04-26 23:07 ` [PATCH v2 0/3] allow checkout and branch to create branches on a merge base Junio C Hamano
2019-04-26 23:40 ` Denton Liu [this message]
2019-04-26 23:52 ` Denton Liu
2019-04-27 0:01 ` Junio C Hamano
2019-04-27 12:02 ` [PATCH v3 0/2] " Denton Liu
2019-04-27 12:02 ` [PATCH v3 1/2] t2018: cleanup in current test Denton Liu
2019-04-27 12:02 ` [PATCH v3 2/2] branch: make create_branch accept a merge base rev Denton Liu
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).