Re: [PATCH v2 2/2] config: Add hashtable for config parsing & retrieval

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 16, 2014, 17:35 UTC
Message-ID: <vpqlhswzriw.fsf@anie.imag.fr>
In-Reply-To: <539F293D.9000602@gmail.com>

Tanay Abhra <tanayabh@gmail.com> writes:

Show 16 quoted lines

> On 06/16/2014 10:11 AM, Matthieu Moy wrote:
>> Tanay Abhra <tanayabh@gmail.com> writes:
>> 
>>> Add a hash table to cache all key-value pairs read from config files
>>> (repo specific .git/config, user wide ~/.gitconfig and the global
>>> /etc/gitconfig). Add two external functions `git_config_get_string` and
>>> `git_config_get_string_multi` for querying in a non-callback manner from the
>>> hash table.
>> 
>> This describes rather well _what_ your patch does, but the most
>> important part of a commit message is to justify _why_ the change is
>> good, and why the way you implemented it is good.
>> 
>> Think of it as an way to convince reviewers to accept your patch.
>
> Okay, but isn't the content of the cover letter is doing that for now.

The cover letter won't be part of the Git history, while the commit messages are.

Imagine someone finding your functions in the code and wondering "wtf is this code doing here?". "git blame" will point this person to your commit message, but digging the mail archives is one big extra step (that essentially no one will make).

> Yeah, I have run the experiments. I will add a test file for it. I should have
> appended it to this series only, my fault. :) A stray observation, Git has very less
> unit tests, compared to the comprehensive test directory for commands.

Yes. But in most cases, code written in a commit is directly reachable from the command-line UI, and can be tested this way.

(I do believe that Git would benefit from more unit-testing, but that's another topic).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/