From: Jeff King <peff@peff.net>
To: Jonathon Mah <me@jonathonmah.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCHv2 1/2] t5304-prune: demonstrate bug in pruning alternates
Date: Mon, 2 Feb 2015 13:41:16 -0500 [thread overview]
Message-ID: <20150202184115.GA25421@peff.net> (raw)
In-Reply-To: <0BD44E44-686B-44B2-A4C0-9E14A99BA96B@jonathonmah.com>
On Mon, Feb 02, 2015 at 10:33:02AM -0800, Jonathon Mah wrote:
> Signed-off-by: Jonathon Mah <me@JonathonMah.com>
> ---
> Adjust prune test directly, much nicer.
Agreed, this is much nicer. A few comments:
> +test_expect_success 'prune: handle alternate object database' '
This test fails, so we either need expect_failure here, or it just needs
to be squashed in with the fix (I generally prefer the latter).
> + test_create_repo A && cd A &&
We generally prefer to chdir in a subshell, so that a failure in the
test does not leave further tests in a confusing spot. Like:
test_create_repo A &&
(
cd A &&
... do stuff in repo ...
# no need to cd ..
) &&
.. do stuff outside repo ...
> + echo "Hello World" > file1 &&
Style nit: we prefer ">file1" with no space.
> + git add file1 &&
> + git commit -m "Initial commit" file1 &&
> + cd .. &&
> + git clone -l -s A B && cd B &&
"-l" is a noop these days. I don't think it is hurting, but I'd prefer
not to propagate bad habits in our tests.
> diff --git a/t/t5710-info-alternate.sh b/t/t5710-info-alternate.sh
> index 5a6e49d..d82844a 100755
> --- a/t/t5710-info-alternate.sh
> +++ b/t/t5710-info-alternate.sh
We can drop this change, then, right?
-Peff
prev parent reply other threads:[~2015-02-02 18:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-02 18:33 [PATCHv2 1/2] t5304-prune: demonstrate bug in pruning alternates Jonathon Mah
2015-02-02 18:34 ` [PATCHv2 2/2] sha1_file: fix iterating loose alternate objects Jonathon Mah
2015-02-02 18:41 ` Jeff King [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=20150202184115.GA25421@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=me@jonathonmah.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).