git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
karthik nayak <karthik.188@gmail.com>
Date
Mar 9, 2015, 12:56 UTC
Message-ID
<54FD9898.8040801@gmail.com>
In-Reply-To
<CAPig+cT-AG2pnHJnkUcnHXzWu9jp2Q7ZBE7Re5jimFU7mOWLhQ@mail.gmail.com>
On 03/09/2015 03:55 AM, Eric Sunshine wrote:
Show 46 quoted lines
> 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

Previous: Eric SunshineNext: Karthik Nayak
Message 4 of 18 in “cat-file: add "--literally" option”
  1. 0/3 cat-file: add "--literally" optionkarthik nayak, Mar 5, 2015
  2. 1/3 cache: modify for "cat-file --literally -t"Karthik Nayak, Mar 5, 2015
  3. Eric SunshineMar 8, 2015
  4. karthik nayakMar 9, 2015
  5. 2/3 sha1_file: implement changes for "cat-file --literally -t"Karthik Nayak, Mar 5, 2015
  6. Junio C HamanoMar 5, 2015
  7. karthik nayakMar 6, 2015
  8. Junio C HamanoMar 6, 2015
  9. karthik nayakMar 7, 2015
  10. Junio C HamanoMar 8, 2015
  11. karthik nayakMar 8, 2015
  12. Junio C HamanoMar 8, 2015
  13. karthik nayakMar 8, 2015
  14. Junio C HamanoMar 8, 2015
  15. karthik nayakMar 9, 2015
  16. 3/3 cat-file: add "--literally" optionKarthik Nayak, Mar 5, 2015
  17. Eric SunshineMar 8, 2015
  18. karthik nayakMar 9, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.