Re: [PATCH 1/2] transport-helper: report errors properly

[Jeff King <peff@peff.net>]

On Thu, Apr 11, 2013 at 11:49:11AM -0500, Felipe Contreras wrote:

> > I am OK with adding the test for import as a separate patch. What I am
> > not OK with (and this goes for the rest of the commit message, too) is
> > failing to explain any back-story at all for why the change is done in
> > the way it is.
> >
> > _You_ may understand it _right now_, but that is not the primary
> > audience of the message. The primary audience is somebody else a year
> > from now who is wondering why this patch was done the way it was.
> 
> Who would be this person? Somebody who wonders why this test is using
> "feature done"? I doubt such a person would exist, as using this
> feature is standard, as can be seen below this chunk. *If* the test
> was *not* using this "feature done", *then* sure, an explanation would
> be needed.

If it was so obvious, why did your initial patch not use "feature done"?
If it was so obvious, why did our email discussion go back and forth so
many times before arriving at this patch?

It was certainly not obvious to me when this email thread started. So in
response to your question: *I* am that person. I was him two weeks ago,
and there is a good chance that I will be him a year from now. Much of
my work on git is spent tracking down bugs in older code, and those
commit messages are extremely valuable to me in understanding what
happened at the time.

But I give up on you. I find most of your commit messages lacking in
details and motivation, making assumptions that the reader is as
familiar with the code when reading the commit as you are when you wrote
it. I tried to help by suggesting in review that you elaborate. That
didn't work. So I tried to help by writing the text myself. But clearly
I am not going to convince you that it is valuable, even if it requires
no work at all from you, so I have nothing else to say on the matter.

-Peff