Re: git-subtree Ready #2
From: David A. Greene <hidden>
Date: 2016-06-15 22:53:06
Junio C Hamano [off-list ref] writes:
Jeff King [off-list ref] writes: It sounds like the simplest and cleanest would be to treat it as if its current version came as a patch submission, cook it just like any other topic in 'pu' down to 'next' down to eventually 'master', with the usual review cycle of pointing out what is wrong and needs fixing followed by a series of re-rolls.
Ok, but we will preserve the history via the subtree merge, yes?
The total amount of change does not look too bad, either:
Yes, it's a fairly small tool.
It does look like it needs to start its life in contrib/ if we were to put this in git.git.
That sounds good to me. It should get a good shakedown before graduating.
I haven't looked at the script fully, but it has an issue from its first line, which is marked with "#!/bin/bash". It is unclear if it is infested by bash-isms beyond repair (in which case "#!/bin/bash" is fine), or it was written portably but was marked with "#!/bin/bash" just by inertia. A patch that corresponds to the above diffstat immediately shows many style issues including trailing eye-sore whitespaces.
Ok.
It seems that it is even capable of installing from contrib/subtree, so keeping it in contrib/ while many issues it may have gets fixed would not hurt the original goal of giving the script more visibility.
Right, I intentially designed it that way.
The change to t/test-lib.sh should be made independent of this topic, I would think.
Ok, I'll propose those changes separately. They are a prerequisite for a git-subtree that is easily testable while in contrib.
quoted hunk
----------------------------------------------------------------diff --git a/t/test-lib.sh b/t/test-lib.sh index e28d5fd..c877a91 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh@@ -55,6 +55,7 @@ unset $(perl -e ' .*_TEST PROVE VALGRIND + BUILD_DIR )); my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env); print join("\n", @vars);@@ -924,7 +925,15 @@ then # itself. TEST_DIRECTORY=$(pwd) fi -GIT_BUILD_DIR="$TEST_DIRECTORY"/.. + +if test -z "$GIT_BUILD_DIR" +then + echo Here + # We allow tests to override this, in case they want to run tests + # outside of t/, e.g. for running tests on the test library + # itself. + GIT_BUILD_DIR="$TEST_DIRECTORY"/.. +fi if test -n "$valgrind" then ----------------------------------------------------------------This change deserves its own justification.
I'll put a patch together with a more extensive explanation. Basically, tests run outside of the top-level t/ directory don't work because there are all sort of assumptions in test-lib.sh about where they live. There are comments in test-lib.sh indicating that it should support tests in other directories but I could not make it work out of the box.
After looking at the history of subtree branch there, however, I agree
that it would not help anybody to have its history in my tree with log
messages like these (excerpt from shortlog output):
update todo
Some todo items reported by pmccurdy
todo
Docs: when pushing to github, the repo path needs to end in .git
todo
todo^
todo
todo: idea for a 'git subtree grafts' command
Ok, these are Avery's commits. I don't know that I have enough context
to improve the logs but I will look throught revisions and try to figure
things out. Avery, could you be of any help here? It sounds like we
need more descriptive log messages.
-Dave