Erick Mattos [off-list ref] writes:
To make the new orphan branch ready to have a reflog on that config
was as simple as creating a "touch file" for reflog. This is the goal
achieved by code.
I have to say that you are somewhat confused about the _goal_ then. touch
is not a goal, it is means to a goal.
It does not matter how you implement the user visible effect, be it a
creation of an empty file, or some other means [*1*]. What matters is
that the user won't get a reflog for a branch that really didn't get
created and must-fail "rev-parse --verify" test checks that.
Another thing that could matter would be that future actions that want to
create a reflog for the same branch (perhaps after the user switches to
'master', another attempt is made to create eta with "checkout -b eta") or
another branch with a similar or related name (say "eta/real") are not get
broken by whatever you do to implement the "we want to create a reflog
when a ref is actually made but not right now" feature. Perhaps the right
way to test that would be to actually try to run such operations and make
sure they do not fail.
[Footnote]
*1* For example, you could have implemented the feature by adding a config
item in ".git/config: [branch "eta"] need-to-create-reflog", and taught
refs.c::update_ref() to pay attention to it (I am not saying that it would
be a better implementation).
2010/7/22 Junio C Hamano [off-list ref]:
Erick Mattos [off-list ref] writes:
quoted
To make the new orphan branch ready to have a reflog on that config
was as simple as creating a "touch file" for reflog. This is the goal
achieved by code.
I have to say that you are somewhat confused about the _goal_ then. touch
is not a goal, it is means to a goal.
I believe you have understood the whole idea of my previous email even
though _words_ means a lot to you. :-1
I meant that that was the code goal. It was the implementation
objective which by itself was the solution chosen for the real
problem.
It does not matter how you implement the user visible effect, be it a
creation of an empty file, or some other means [*1*]. What matters is
that the user won't get a reflog for a branch that really didn't get
created and must-fail "rev-parse --verify" test checks that.
Another thing that could matter would be that future actions that want to
create a reflog for the same branch (perhaps after the user switches to
'master', another attempt is made to create eta with "checkout -b eta") or
another branch with a similar or related name (say "eta/real") are not get
broken by whatever you do to implement the "we want to create a reflog
when a ref is actually made but not right now" feature. Perhaps the right
way to test that would be to actually try to run such operations and make
sure they do not fail.
You were cutting off redundancy checks, weren't you? If there is no
reflog file (tested by "test -f"), consequently no reflog (tested by
"rev-parse --verify") and no branch with the chosen name, certainly
someone can create one the "almost used" name, once there is no any
difference from a normal situation.
[Footnote]
*1* For example, you could have implemented the feature by adding a config
item in ".git/config: [branch "eta"] need-to-create-reflog", and taught
refs.c::update_ref() to pay attention to it (I am not saying that it would
be a better implementation).
About the late parenthesis: thank God! ;-)
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.
With those two patch lines of mine "--orphan with -l and
core.logAllRefUpdates=false" testing script is finished.
Finally: you are the man in charge so I would really like to enforce
that if you need me to do anything more I will be _really_ glad to
help. I love git and everything good done to it it is done to me too
as one of its daily user.
Best regards
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
Hi,
2010/8/24 Jonathan Nieder [off-list ref]
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.
You're welcome! It is my pleasure, really! The reason for me to start
participating is a concern to pay back some of the benefits I have
been having by using free software.
I wish I could help all the excellent programs I use but I intend to
help at least some. Nonetheless I would like to do it to all.
So I am really grateful that you let me know my work was useful to you.
I have been addressing problems I had when dealing with git and
--orphan is important for private branch working. Even that I use
--reset-author more often.
I would eventually add more stuff to git, while it is already quite a
wonderful tool right now though It can be enhanced. However I don't
plan to because I had been facing extreme opposition and I don't like
to disturb. I only want to spend my effort where it helps.
Erick Mattos wrote:
quoted
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.
Tests won't impede a developer but it will show him that he is
changing something in fact. And they will tell him he needs to adjust
the tests too.
But in general it is a good practice to adhere to that principle
unless implementation is what you are really testing. At this case,
all right behavior had their tests before it and this one is to check
for a bad implementation.
"The problem", test -f, is used 428 times inside 92 scripts and in a
similar way (test -f .git/...) 48 times inside 19 scripts. So I don't
think it is correct that I have to struggle so much to use it too.
And the major fact is that the actual test is testing NOTHING and that
the test lamed IS important!
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?
I really haven't got you point. Let's see:
'giving up --orphan not committed when -l and core.logAllRefUpdates =
false deletes reflog'
Don't you think reflog deletion is immediately obvious?
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}
Your test is not the same and that is already done by zeta: '--orphan
with -l makes reflog when core.logAllRefUpdates = false'.
Happily, I am not the man in charge, so feel free to take my words
at whatever value you choose. :)
Not quite happily though.
Unfortunately the problem which git does not address for now is that
git have cemented the distributed repository but it really haven't
created a distributed development workflow.
Someone still have to create a workflow where people merge code,
design, give the directions, do quality optimization and more managing
things TOGETHER.
While everybody have the right to access and work separately, there is
no existent tool to integrate the work without a need for a moderator
which is always a flow constriction.
You can see Git and Linux as example: while the widespread use of
distributed repositories, every management is closed to the
maintainers which can possibly ignore good stuff or hasty accept later
proved bad deals.
That dictatorial guidance leads to political problems, inherent of
human being existence, putting code subscribed to who is writing it
and not to how good it is.
I think that is the major reason for so much forks and developer
desertions nowadays and for not merging potential good stuff because
of people animosity.
I believe someone very clever will soon find a way for not depending
so much of one person alone and get a way of making code evolution
happen by itself in a real distributed work flow.
Regards,
Jonathan
Your words value much. So I would appreciate to know your following perceptions.
It is always a pleasure to talk to you and to have an opportunity to
get your always objective and balanced point-of-views.
Best regards