Bert Wesarg <bert.wesarg@xxxxxxxxxxxxxx> writes: > On Tue, Dec 1, 2009 at 01:21, Junio C Hamano <gitster@xxxxxxxxx> wrote: > ... >> Hmm, the Subject: matches what the code does, but nobody mentions why it >> is necessary (iow, what breaks in the current code and what becomes better >> if the patch is applied). The blank space before your "S-o-b:" line is >> for you to describe these things. > Sure. unfortunately the code where the string list is filled is not in > the patch. But if you look at the code it should be self-explanatory. That is _exactly_ why I want the description in the commit log message. I don't want commit messages (or lack thereof) to force people to look at the code outside the patch. Otherwise I'll have to ask _you_ to personally give the 7-line explanation you just gave us to anybody who runs "git log -p" with the default context size after this patch is applied. I do not think you have the bandwidth to handle that ;-). > There is actually also an other solution: we could first strdup the > ref name to the .util member and take this as the input for the > abbrev_ref()/string list entry and safe there the strdup. I too thought something like that may make sense, but it doesn't look like so, for two reasons: - string-list API is a bit cumbersome to use if you allocate the string yourself. You will be made responsible for freeing them, and often you do so by setting strdup_strings to true immediately before calling string_list_clear(), which is kind of ugly; - The ref abbrev_branch() is called and the address of whose substring is taken to be used as "name" in handle_one_branch() is refspec.src, but what goes to .util is refname that is refspec.dst---they are different strings and one is not a substring of the other. IOW, your patch text is good; it just would have been better with a bit more explanation. Thanks, will queue in 'maint'. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html