Thread (4 messages) flat view 4 messages, 3 authors, 2016-06-15

Re: [PATCH] t2017: redo physical reflog existance check

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:22

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help