Re: [PATCH v5 11/15] remote-testgit: make clear the 'done' feature

From: Max Horn <max@quendi.de>
Date: Nov 12, 2012, 11:20 UTC
Message-ID: <EA56F0CC-7C93-491F-A076-4A1AA9593ED0@quendi.de>
In-Reply-To: <CAMP44s0o1eP+aeT0AHu4uP1NPLqJq56qUDb-+F_x5NjoJCnf+A@mail.gmail.com>

On 11.11.2012, at 22:22, Felipe Contreras wrote:

Show 11 quoted lines

> On Sun, Nov 11, 2012 at 9:49 PM, Max Horn <max@quendi.de> wrote:
>> 
>> On 11.11.2012, at 14:59, Felipe Contreras wrote:
>> 
>>> People seeking for reference would find it useful.
>> 
>> Hm, I don't understand this commit message. Probably means I am j git fast-export --use-done-featureust too dumb, but since I am one of those people who would likely be seeking for reference, I would really appreciate if it could clarified. Like, for example, I don't see how the patch below makes anything "clear", it just seems to change the "import" command of git-remote-testgit to make use of the 'done' feature?
> 
> No, the done feature was there already, but not so visible: git
> fast-export --use-done-feature <-there. Which is the problem, it's too
> easy to miss, therefore the need to make it clear.

Aha, now I understand what this patch is about. So I would suggest this alternate commit message:

  remote-testgit: make it explicit clear that we use the 'done' feature

  Previously we relied on passing '--use-done-feature ' to git fast-export, which is
  easy to miss when looking at this script. Since remote-testgit is also a reference
  implementation, we now explicitly output 'feature done' / 'done' to make it
  crystal clear that we implement this feature.

Or perhaps a little bit less verbose. With a commit message like the above, I think I would have grokked the patch right away. With the original message, that was not the case (else I wouldn't have wrote my initial email). And even though I now understand (or at least believe to understand) the patch, I don't think the original message is that helpful... indeed, "make clear the 'done' feature" is ambiguous. You meant it as "make clear the 'done' feature is implemented / used", while I understood it as "make clear what the 'done' feature is about". Looking at the patch can help to resolve that, but (a) my wrong interpretation threw me off-track and (b) I thought that the point of commit messages was to give an overview of a patch without having to look at it... So at the very least, the message should explain what exactly is "made clear".

Anyway, a small change to the commit message hopefully will not be a problem. :-)

Cheers, Max