On Thu, Jul 26, 2012 at 09:34:27AM -0700, Junio C Hamano wrote:
> >> not ok - 21 committer is automatic
> [...]
> > I am not sure that the test is really all that useful. The point seems
> > to be that we fall back to some kind of system-based ident, but that is
> > not portable.
>
> I think the point is to make sure that the "# Committer:" line is
> given to the reader to remind that we took the codepath that comes
> up with a committer ident by using untrustworthy heuristics. You
> are correct that the usefulness of the value of system-based ident
> varies between systems (that is why it is stripped out with sed),
> though.
Ah, right. I was led astray by the crappy test title. When viewed with
the test immediately prior (which checks that "Author:" is shown in the
template), it makes more sense.
> You earlier gave a reason why f20f387 (commit: check committer identity
> more strictly, 2012-07-23) does not have a test for it; I think the
> same reason applies why this test is unworkable.
Right. You can check this only when "git var GIT_COMMITTER_IDENT" works,
and you can check the f20f387 behavior only when it does _not_ work. So
we could do something like:
(sane_unset GIT_COMMITTER_NAME &&
sane_unset GIT_COMMITTER_EMAIL &&
git var GIT_COMMITTER_IDENT >/dev/null) &&
test_set_prereq AUTOIDENT ||
test_set_prereq NOAUTOIDENT
test_expect_success AUTOIDENT \
'mention auto ident in commit template'
'...'
test_expect_success NOAUTOIDENT \
'git rejects bogus ident before starting editor'
'...'
But it is somewhat unsatisfying to only get random test coverage
depending on how your system happens to be configured. I guess we
somewhat have that already with the case-insensitivity tests.
Do we want to go that route, or just drop this test completely?
> A related tangent; all the test vectors in this script seems to be
> too wide, and we probably would want to narrow them for what each
> test wants to see. For example, the test in question only wants to
> see "# Committer: <some system based ident>" and it does not matter
> if the template was rewritten in future versions of Git so that it
> does not begin with "# Please enter...". Similarly, the one
> previous only wants to see "# Author: <different from committer>".
Agreed. They should probably just i18ngrep for "^# Committer: " or
similar.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html