Re: [PATCH 1/1] general style: replaces memcmp() with starts_with()

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 18, 2014, 19:23 UTC
Message-ID: <xmqqiorb8g5o.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAPig+cSu=zgXdd55dj-u6mOzHqpUme7r-7OH+jjnOEt2P6C0vw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 21 quoted lines

>>> A patch of this nature doesn't require much more description than
>>> stating what it does ("replace memcmp() with starts_with()") and why
>>> ("improve code clarity"). The following rewrite might be sufficient:
>>>
>>>     Subject: replace memcmp() with starts_with()
>>>
>>>     starts_with() indicates the intention of the check more clearly
>>>     than memcmp().
>>
>> This is more concise; thank you. I will adapt this as the message for
>> the next version of this patch. Would it be wise to mention magic
>> numbers, as the discussion surrounding the rationale of this patch,
>> especially with Junio and Michael, has centered around that?
>
> I was thinking of mentioning magic numbers in the example, but decided
> that their removal was a natural and obvious consequence of the
> improvement to code clarity, so it wasn't strictly necessary to talk
> about it. On the other hand, it is a good secondary justification,
> thus it should be perfectly acceptable to mention it. If I was writing
> the commit message, I might start by saying "As an additional
> benefit..." and then talk a bit about magic number retirement.

I think your subject is good (as you said, it makes it clear that it is a project-wide clean-up by not mentioning any specific area), but "indicates the intention of the check more clearly" would not tell new readers who are unfamiliar with the helper API what "intention" it is talking about very much, so perhaps

	Subject: use starts_with() instead of !memcmp()

	When checking if a string begins with a constant string,
	using starts_with() is less error prone than calling
	!memcmp() with an explicit byte count.

or something?