All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Tay Ray Chuan <rctay89@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] http: add_fill_function checks if function has been added
Date: Sat, 07 Mar 2009 13:49:57 -0800	[thread overview]
Message-ID: <7vd4ctm29m.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <be6fef0d0903071249s42ac7f94o82461ca32dcdfcd5@mail.gmail.com> (Tay Ray Chuan's message of "Sun, 8 Mar 2009 04:49:13 +0800")

Tay Ray Chuan <rctay89@gmail.com> writes:

> Calling a fill function repeatedly won't break behaviour, because fill
> functions (those that are currently defined in git) are designed to be
> called repeatedly. But it's just useless to call the same fill
> function repeatedly without any reason.
>
> So should I still address the "THIS and THAT breakages"?

Your above explanation is good, but it was given after I asked ;-).
Making unnecessary call repeatedly counts as a breakage your patch fixes,
right?

I didn't look at the callers of add_fill_function(), but "fill" takes a
callback data and different invocation of add_fill_function() could be
passing different callback data.  In such a case, doesn't it feel wrong to
omit the "duplicated" calls to register the fill callback?  Your patch
makes me suspect that it _might_ be better to fix the callers not to call
the function repeatedly when they know they only want one-shot invocation.

  reply	other threads:[~2009-03-07 21:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-07 12:21 [PATCH] http: add_fill_function checks if function has been added Tay Ray Chuan
2009-03-07 20:18 ` Junio C Hamano
2009-03-07 20:49   ` Tay Ray Chuan
2009-03-07 21:49     ` Junio C Hamano [this message]
2009-03-08 10:27       ` Tay Ray Chuan
2009-03-08 19:38         ` Junio C Hamano
2009-03-09 12:01           ` Tay Ray Chuan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vd4ctm29m.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=rctay89@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.