Re: [PATCH] use write_str_in_full helper to avoid literal string lengths

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 12, 2009, 23:56 UTC
Message-ID: <7veiqbbttg.fsf@alter.siamese.dyndns.org>
In-Reply-To: <87skes35mf.fsf@meyering.net>

Jim Meyering <jim@meyering.net> writes:

Show 26 quoted lines

>  ...Thus not requiring the added allocation, and still avoiding
> the maintenance risk of literal string lengths.
> These days, compilers are good enough that strlen("literal")
> imposes no run-time cost.
>
> Transformed via this:
>
>     perl -pi -e \
>         's/write_in_full\((.*?), (".*?"), \d+\)/write_str_in_full($1, $2)/'\
>       $(git grep -l 'write_in_full.*"')
>
>
> From fe368f8b3720f04c9dfce952711d2fb412b52e3c Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering@redhat.com>
> Date: Sat, 12 Sep 2009 09:56:13 +0200
> Subject: [PATCH] use write_str_in_full helper to avoid literal string lengths
>
> * cache.h (write_str_in_full): Define function.
> * builtin-fetch.c (quickfetch): Use it.
> * builtin-reflog.c (expire_reflog): Likewise.
> * commit.c (write_shallow_commits): Likewise.
> * config.c (git_config_set_multivar): Likewise.
> * rerere.c (write_rr): Likewise.
> * transport-helper.c (get_helper, disconnect_helper): Likewise.
> (get_refs_list): Likewise.
> * upload-pack.c (receive_needs): Likewise.

Thanks. I agree with the reasoning you wrote outside the proposed log message.

We usually do not write these bullet points (iow, we are not GNU) in our log message. The names of the functions, call sites and files that are involved are something anybody can see from the patch text,

I think the GNU convention was useful back when we were trapped in a system with non-atomic commits, where it was very hard to see what files were affected in a single logical changeset (i.e. CVS).

Luckily, we graduated those dark ages.

Instead, we prefer to have justifications (and methods), like what you wrote at the beginning of your message. These are not something people can find in the patch text and they deserve to be recorded in the commit.