From: karthik nayak Date: Mon, 09 Mar 2015 12:56:56 GMT Subject: Re: [PATCH v3 1/3] cache: modify for "cat-file --literally -t" Message-ID: <54FD9898.8040801@gmail.com> In-Reply-To: On 03/09/2015 03:55 AM, Eric Sunshine wrote: > On Thu, Mar 5, 2015 at 1:18 PM, Karthik Nayak 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 >> --- >> 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