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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tanay Abhra <tanayabh@xxxxxxxxx> writes:

> On 06/16/2014 10:11 AM, Matthieu Moy wrote:
>> Tanay Abhra <tanayabh@xxxxxxxxx> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]