Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident

[Jeff King <peff@peff.net>]

On Thu, Nov 15, 2012 at 03:13:47AM -0800, Jeff King wrote:

> I think a much more compelling argument/commit message for your
> suggested patch would be:
> 
>   We currently prompt the user for the "From" address. This is an
>   inconvenience in the common case that the user has configured their
>   identity in the environment, but is meant as a safety check for when
>   git falls back to an implicitly generated identity (which may or may
>   not be valid).
> 
>   That safety check is not really necessary, though, as by default
>   send-email will prompt the user for a final confirmation before
>   sending out any message. The likelihood that a user has both bothered
>   to turn off this default _and_ not configured any identity (nor
>   checked that the automatic identity is valid) is rather low.

If that is the route we want to go, then we should obviously drop my
series in favor of your final patch. I think it would also need a test
update, no?

I think a more concise commit message would help, too. I disagree with a
great deal of the reasoning in your existing message, but those parts
turn out not to be relevant. The crux of the issue is that the safety
check is not necessary because there is already one (i.e., point 8 of
your list).  Feel free to use any or all of my text above.

>From my series, there were a few cleanups that might be worth salvaging.
Here is a rundown by patch:

  [1/8]: test-lib: allow negation of prerequisites

This stands on its own, and is something I have wanted a few times in
the past. However, since there is no immediate user, I don't know if it
is worth doing or not.

  [2/8]: t7502: factor out autoident prerequisite

A minor cleanup and possible help to future tests, but since there are
no other callers now, not sure if it is worth it.

  [3/8]: ident: make user_ident_explicitly_given static

A cleanup that is worth doing, I think.

  [4/8]: ident: keep separate "explicit" flags for author and committer

Another cleanup.  This is "more correct", in that it handles the corner
cases I mentioned in the commit message. But no current code cares about
those corner cases, because the only real caller is git-commit, and this
is a purely internal interface. I could take or leave it.

  [5/8]: var: accept multiple variables on the command line

The tests for this can be split out; we currently don't have "git var"
tests at all, so increasing our test coverage is reasonable. The
multiple variables thing is potentially useful, but there are simply not
that many callers of "git var", and nobody has been asking for such a
feature (we could use it to save a process in git-send-email, but it is
probably not worth the complexity).

  [6/8]: var: provide explicit/implicit ident information
  [7/8]: Git.pm: teach "ident" to query explicitness

These two should probably be dropped. They would lock us into supporting
the explicit/implicit variables in "git var", for no immediate benefit.

  [8/8]: send-email: do not prompt for explicit repo ident

Obviously drop.

> I could accept that line of reasoning.  I see that this argument is
> buried deep in your commit message, but I will admit to not reading your
> 9-point list of conditions all that closely, as the first 7 points are,
> in my opinion, not relevant (and I had already read and disagreed with
> them in other messages).

If it sounds like I am blaming you here, I am to some degree. But I am
also blaming myself. I should have read your commit message more
carefully, and I'm sorry for not doing so. I hope we can both try harder
to avoid getting side-tracked on arguing about issues that turned out
not to be important at all (of course, we cannot know which part of the
discussion will turn out to be important, but I think there some
obviously unproductive parts of this discussion).

-Peff