From: Matthieu Moy <Matthieu.Moy@grenoble-inp•fr>
To: Tanay Abhra <tanayabh@gmail•com>
Cc: git@vger•kernel.org, Ramkumar Ramachandra <artagnon@gmail•com>,
Eric Sunshine <sunshine@sunshineco•com>
Subject: Re: [PATCH v2 2/2] config: Add hashtable for config parsing & retrieval
Date: Mon, 16 Jun 2014 19:35:35 +0200 [thread overview]
Message-ID: <vpqlhswzriw.fsf@anie.imag.fr> (raw)
In-Reply-To: <539F293D.9000602@gmail.com> (Tanay Abhra's message of "Mon, 16 Jun 2014 10:28:29 -0700")
Tanay Abhra <tanayabh@gmail•com> writes:
> 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/
next prev parent reply other threads:[~2014-06-16 17:35 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-16 8:27 [PATCH v2 0/2] Git config cache & special querying api utilizing the cache Tanay Abhra
2014-06-16 8:27 ` [PATCH v2 1/2] string-list: Add string_list initializer helper functions Tanay Abhra
2014-06-16 22:59 ` Junio C Hamano
2014-06-17 19:05 ` Tanay Abhra
2014-06-17 22:10 ` Junio C Hamano
2014-06-16 8:27 ` [PATCH v2 2/2] config: Add hashtable for config parsing & retrieval Tanay Abhra
2014-06-16 17:11 ` Matthieu Moy
2014-06-16 17:28 ` Tanay Abhra
2014-06-16 17:35 ` Matthieu Moy [this message]
2014-06-17 5:34 ` Jeff King
2014-06-17 5:46 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=vpqlhswzriw.fsf@anie.imag.fr \
--to=matthieu.moy@grenoble-inp$(echo .)fr \
--cc=artagnon@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=sunshine@sunshineco$(echo .)com \
--cc=tanayabh@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox