Re: [PATCH v2] pull: handle --log=<n>
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:04:50
Junio C Hamano [off-list ref] writes:
A proper patch series would probably be
[1/N] Teach "test_commit --tag" and replace existing "test_commit"
with "test_commit --tag"
[2-N/N] For all the test scripts, analyse and judge if they are
better off with the auto-generated tags (i.e. no change wrt
the result of 1/N) or tags that are created by the script
at strategic places only as needed, and convert those that
are better read without "test_commit --tag".
[1/N] would be mechanical and easy, but justifying the change in the
remainder would be a lot of work and reviewing would be, too, and
would require a good taste.
Actually even the above is not sufficient, as we have to worry about
topics in-flight (aka "the rest of the world is not standing still
waiting for you"). It must go more like this:
[1/N] Introduce a new helper (let's call it "make_commit") that
by default does not do too much but optionally can do as much
as the "test_commit" does. Reimplement "test_commit" in
terms of "make_commit"
[2-N/N] Exaine existing uses of "test_commit" and reduce what they
do by rewriting them with "make_commit" as needed.
So I'd say 1/N may probably be something worth doing, but I am not
convinced that the rest is a good use of reviewer's time.
Speaking of "does not do too much", I think the behaviour of
"test_commit" that always adds new a file outlived its usefulness.
When a test is primarily interested in the topology of the resulting
history (most "log" tests are), we can use "commit --allow-empty"
these days (I think "test_commit" predates it and adding a file may
have been a handy way to ensure that the new commit's tree is
different from parent's). And when a test is primarily about how
branches with different contents in various paths interact with each
other (e.g. checking out, merging), it does not make a very useful
test vector to work on if you can only build your tests with a set
of branches whose differences can only be creation and deletion but
never in-place modification of paths.
Among various potential uses of the hypothetical "test_commit"
replacement that I would imagine, what is common is only that it
does the "tick" thing and then it creates a commit, and nothing
else. Everything else is optional and needs customization. But
then there is not much left, i.e.
make_commit () {
test_tick && git commit --allow-empty "$@"
}
which may be useful to avoid forgetting the tick thing, but does not
help much else.
Even if that were found to be useful, this discussion is only for
step 1/N; let's refrain from rewriting everything for the sake of
rewriting. I do not think it would be a good use of developer time
or reviewer time.