Re: [PATCH v3 1/3] cache: modify for "cat-file --literally -t"

[karthik nayak <karthik.188@gmail.com>]

On 03/09/2015 03:55 AM, Eric Sunshine wrote:
> On Thu, Mar 5, 2015 at 1:18 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> cache: modify for "cat-file --literally -t"
>
> It is desirable for the first line of the commit message to explain,
> as well as possible, the intent of the patch. The bulk of the commit
> message then elaborates. Unfortunately, this line says almost nothing.
> All patches modify, so writing "modify" here is not helpful and merely
> wastes precious horizontal real estate. A more informative summary
> might say something like:
>
>      cache: add object_info::typename in support of 'cat-file --literally'
>
>> Add a "struct strbuf *typename" to object_info to hold the
>> typename when the literally option is used. Add a flag to
>> notify functions when literally is used.
>
> It's good to split up changes such that each patch comprises one
> logical step, however, this patch does not really do anything on its
> own, so having it stand-alone doesn't make much sense. It would make
> more sense to fold it into the patch which actually requires these
> changes.
>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> diff --git a/cache.h b/cache.h
>> index 4d02efc..949ef4c 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -830,6 +830,7 @@ extern int is_ntfs_dotgit(const char *name);
>>
>>   /* object replacement */
>>   #define LOOKUP_REPLACE_OBJECT 1
>> +#define LOOKUP_LITERALLY 2
>>   extern void *read_sha1_file_extended(const unsigned char *sha1, enum object_type *type, unsigned long *size, unsigned flag);
>>   static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)
>>   {
>> @@ -1296,6 +1297,7 @@ struct object_info {
>>          unsigned long *sizep;
>>          unsigned long *disk_sizep;
>>          unsigned char *delta_base_sha1;
>> +       struct strbuf *typename;
>>
>>          /* Response */
>>          enum {
>> --
>> 2.3.1.167.g7f4ba4b.dirty

Hey Eric!
Thanks for the feedback, I guess I need to stick to different patches 
only for logical changes, not considering different files.
Have been reading old commit messages to get a hang of it. Also found 
some blog posts explaining why we should use imperative sentences while 
writing Git commit messages. All makes sense now.

Regards
-Karthik