Re: [PATCH] Rewritten fetch-pack.c:filter_refs() using starts_with() instead of memcmp()

[Eric Sunshine <sunshine@sunshineco.com>]

Thanks for the submission. Comments below to give you a taste of the
Git review process...

On Thu, Mar 20, 2014 at 6:04 AM, MustafaOrkunAcar
<mustafaorkunacar@gmail.com> wrote:
> Subject: Rewritten fetch-pack.c:filter_refs() using starts_with() instead of memcmp()

Use imperative mood: "Rewrite" rather than "Rewritten". Mention the
module or function you're touching at the start of the subject,
followed by a colon and space. For example:

    Subject: filter_refs: replace memcmp() with starts_with()

> Hi, I have completed one of the microprojects -14th one: "Change fetch-pack.c:filter_refs() to use starts_with() instead of memcmp()." The only line in the function filter_refs() containing memcmp() is changed with starts_with(). I plan to apply for GSoC 2014. Any feedback is appreciated. Thanks.

Wrap text to 65-70 characters.

This area above your sign-off is where you should explain the purpose
of the patch and justify the change. For a small one like this, you
shouldn't need more than one or two simple sentences.

> Signed-off-by: MustafaOrkunAcar <mustafaorkunacar@gmail.com>
> ---

This area below the "---" line under your sign-off is for commentary
which won't likely be relevant to someone looking at the patch in the
project history months or years from now. Everything you wrote above
about GSoC and only one instance of memcmp() belongs here.

The patch itself looks reasonable. As suggested by the microproject,
were you able to find any other places in the project which could
benefit likewise? If so, perhaps include a few of them when you
resubmit.

>  fetch-pack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index f061f1f..17823ab 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -506,7 +506,7 @@ static void filter_refs(struct fetch_pack_args *args,
>                 int keep = 0;
>                 next = ref->next;
>
> -               if (!memcmp(ref->name, "refs/", 5) &&
> +               if (starts_with(ref->name, "refs/") &&
>                     check_refname_format(ref->name, 0))
>                         ; /* trash */
>                 else {
> --
> 1.9.1.286.g5172cb3