Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Derrick Stolee <stolee@gmail.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>, git@vger.kernel.org
Cc: Derrick Stolee <dstolee@microsoft.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 0/2] Improve Fix code coverage for checkout
Date: Thu, 21 May 2020 08:38:23 -0400
Message-ID: <d5845a16-df61-0497-5af9-75d0174a0fc1@gmail.com> (raw)
In-Reply-To: <20200521020712.1620993-1-sandals@crustytoothpaste.net>

On 5/20/2020 10:07 PM, brian m. carlson wrote:
> Patch #1 reduces the number of options in the scenario which Stolee
> mentioned above.  There's now just a NULL and a non-NULL case, and the
> NULL case is now relatively straightforward and uninteresting.

I'm glad you and Junio were able to simplify this case!

> Patch #2 adds a test for the particular set of options which will
> trigger this case as an independent test.  I didn't think it made sense
> to put this in t0021, since ultimately that set of options isn't about
> conversions and it would seem out of place there, so I put it in t2060.
> 
> I'm ultimately on the fence for this case, because I think it's really a
> corner case and testing this is probably not that interesting, so my
> preference is for us to pick up patch 1 and drop patch 2.  However, I
> added patch 2 in case we do indeed want a test for this, and I'll let
> Junio and others decide on what's best.

I think the test is helpful, since it is not very complicated to set up.

The test you created for t0021-conversion.sh earlier was quite complicated
and required internal knowledge to get going. However, this test only uses
porcelain commands to trigger the conditional.

From the discussion here, it is not obvious that info->commit is ever NULL,
so having the test can prevent a future change from making that same
assumption.

This series is Reviewed-by: Derrick Stolee <dstolee@microsoft.com>

Thanks,
-Stolee

      parent reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-15 17:22 Git Test Coverage Report (v2.27.0-rc0) Derrick Stolee
2020-05-19 12:11 ` Derrick Stolee
2020-05-19 20:07   ` René Scharfe
2020-05-19 23:42   ` brian m. carlson
2020-05-20  1:38     ` brian m. carlson
2020-05-19 18:31 ` [PATCH] t4067: make rename detection test output raw diff Jonathan Tan
2020-05-19 19:00   ` Derrick Stolee
2020-05-20  1:41 ` [PATCH 1/1] builtin/checkout: simplify metadata initialization brian m. carlson
2020-05-20 15:17   ` Junio C Hamano
2020-05-20 22:37     ` brian m. carlson
2020-05-21  2:07 ` [PATCH v2 0/2] Improve Fix code coverage for checkout brian m. carlson
2020-05-21  2:07   ` [PATCH v2 1/2] builtin/checkout: simplify metadata initialization brian m. carlson
2020-05-21 17:35     ` Junio C Hamano
2020-05-23 12:22       ` brian m. carlson
2020-05-21  2:07   ` [PATCH v2 2/2] t2060: add a test for switch with --orphan and --discard-changes brian m. carlson
2020-05-21 12:38   ` Derrick Stolee [this message]

Reply instructions:

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d5845a16-df61-0497-5af9-75d0174a0fc1@gmail.com \
    --to=stolee@gmail.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git