From: Jonathan Nieder <jrnieder@gmail.com>
To: Erick Mattos <erick.mattos@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] t2017: redo physical reflog existance check
Date: Mon, 23 Aug 2010 23:03:47 -0500 [thread overview]
Message-ID: <20100824040347.GA19817@burratino> (raw)
In-Reply-To: <AANLkTin76s-ONFuP+OWdxB5LJNf2D1Du+hKxB2s_WhTa@mail.gmail.com>
Hi Erick,
First, thanks for checkout --orphan. I was a skeptic but now that I
have seen it being used for things similar to "going open source" (and
the related "simplifying logs while working privately on a patch
series") it looks to be a pretty nice tool.
Erick Mattos wrote:
> I don't see a need for so much reluctance: "test -f" is not a taboo
> inside a script in t folder and the added tests don't change anything
> about the design and implementation which IMHO is well fit.
The principle (though we do not always adhere to it) is that test
scripts should pass or fail based only on advertised behavior, not
implementation details. That way, _later_ any person who wants to
improve the implementation will not be impeded by tests.
The behavior that "test -f .git/logs/refs/heads/eta" checks for is not
part of the advertised behavior and though it does affect the
observable behavior, it is not immediately obvious how. Wouldn't it
be best if the test described that advertised behavior while checking
for it?
e.g.:
git config core.logallrefupdates false &&
test_when_finished "git config core.logallrefupdates true" &&
git checkout master &&
git checkout -l --orphan eta &&
test_must_fail git rev-parse --verify eta@{0} &&
test_tick &&
git commit -m "initial commit" &&
git rev-parse --verify eta@{0}
Happily, I am not the man in charge, so feel free to take my words
at whatever value you choose. :)
Regards,
Jonathan
next prev parent reply other threads:[~2010-08-24 4:06 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-22 1:46 [PATCH] t2017: redo physical reflog existance check Erick Mattos
2010-07-22 17:35 ` Junio C Hamano
2010-07-22 18:58 ` Erick Mattos
2010-07-23 2:23 ` Junio C Hamano
2010-07-23 15:04 ` Erick Mattos
2010-08-24 4:03 ` Jonathan Nieder [this message]
2010-08-24 16:57 ` Junio C Hamano
2010-08-24 18:57 ` Erick Mattos
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=20100824040347.GA19817@burratino \
--to=jrnieder@gmail.com \
--cc=erick.mattos@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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).