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

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to